[tarantool-patches] [PATCH 7/8] sql: clean-up affinity from SQL source code
Nikita Pettik
korablev at tarantool.org
Fri Dec 28 12:34:51 MSK 2018
Replace remains of affinity usage in SQL parser, query optimizer and
VDBE. Don't add affinity to field definition when table is encoded into
msgpack. Remove field type <-> affinity converters, since now we can
operate directly on field type.
Part of #3698
---
src/box/sql.c | 6 +-----
src/box/sql/build.c | 52 ------------------------------------------------
src/box/sql/expr.c | 11 +++++-----
src/box/sql/insert.c | 39 ++++++------------------------------
src/box/sql/select.c | 17 +++++-----------
src/box/sql/sqliteInt.h | 53 -------------------------------------------------
src/box/sql/vdbe.c | 4 ++--
src/box/sql/where.c | 10 ----------
src/box/sql/wherecode.c | 29 ++++++++++-----------------
src/box/sql/whereexpr.c | 3 ++-
test/sql/types.result | 15 +++++++-------
test/sql/upgrade.result | 6 +++---
12 files changed, 42 insertions(+), 203 deletions(-)
diff --git a/src/box/sql.c b/src/box/sql.c
index a498cd8fe..8e2f5b6c7 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -997,7 +997,7 @@ sql_encode_table(struct region *region, struct Table *table, uint32_t *size)
uint32_t cid = def->fields[i].coll_id;
struct field_def *field = &def->fields[i];
const char *default_str = field->default_value;
- int base_len = 5;
+ int base_len = 4;
if (cid != COLL_NONE)
base_len += 1;
if (default_str != NULL)
@@ -1009,10 +1009,6 @@ sql_encode_table(struct region *region, struct Table *table, uint32_t *size)
assert(def->fields[i].is_nullable ==
action_is_nullable(def->fields[i].nullable_action));
mpstream_encode_str(&stream, field_type_strs[field->type]);
- mpstream_encode_str(&stream, "affinity");
- enum affinity_type aff =
- sql_field_type_to_affinity(def->fields[i].type);
- mpstream_encode_uint(&stream, aff);
mpstream_encode_str(&stream, "is_nullable");
mpstream_encode_bool(&stream, def->fields[i].is_nullable);
mpstream_encode_str(&stream, "nullable_action");
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 2885bb6d5..9eaa4300a 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -485,58 +485,6 @@ sql_field_retrieve(Parse *parser, Table *table, uint32_t id)
return field;
}
-enum field_type
-sql_affinity_to_field_type(enum affinity_type affinity)
-{
- switch (affinity) {
- case AFFINITY_INTEGER:
- return FIELD_TYPE_INTEGER;
- case AFFINITY_REAL:
- return FIELD_TYPE_NUMBER;
- case AFFINITY_TEXT:
- return FIELD_TYPE_STRING;
- case AFFINITY_BLOB:
- return FIELD_TYPE_SCALAR;
- case AFFINITY_UNDEFINED:
- return FIELD_TYPE_ANY;
- default:
- unreachable();
- }
-}
-
-enum affinity_type
-sql_field_type_to_affinity(enum field_type field_type)
-{
- switch (field_type) {
- case FIELD_TYPE_INTEGER:
- case FIELD_TYPE_UNSIGNED:
- return AFFINITY_INTEGER;
- case FIELD_TYPE_NUMBER:
- return AFFINITY_REAL;
- case FIELD_TYPE_STRING:
- return AFFINITY_TEXT;
- case FIELD_TYPE_SCALAR:
- return AFFINITY_BLOB;
- case FIELD_TYPE_ANY:
- return AFFINITY_UNDEFINED;
- default:
- unreachable();
- }
-}
-
-char *
-sql_affinity_str_to_field_type_str(const char *affinity_str)
-{
- if (affinity_str == NULL)
- return NULL;
- size_t len = strlen(affinity_str) + 1;
- char *type_str = (char *) sqlite3DbMallocRaw(sql_get(), len);
- for (uint32_t i = 0; i < len - 1; ++i)
- type_str[i] = sql_affinity_to_field_type(affinity_str[i]);
- type_str[len - 1] = '\0';
- return type_str;
-}
-
/*
* Add a new column to the table currently being constructed.
*
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index 32606dac3..22b64b526 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -311,8 +311,7 @@ binaryCompareP5(Expr * pExpr1, Expr * pExpr2, int jumpIfNull)
{
enum field_type lhs = sql_expr_type(pExpr2);
enum field_type rhs = sql_expr_type(pExpr1);
- u8 type_mask = sql_field_type_to_affinity(sql_type_result(rhs, lhs)) |
- (u8) jumpIfNull;
+ u8 type_mask = sql_type_result(rhs, lhs) | (u8) jumpIfNull;
return type_mask;
}
@@ -2606,9 +2605,9 @@ exprINAffinity(Parse * pParse, Expr * pExpr)
if (pSelect) {
struct Expr *e = pSelect->pEList->a[i].pExpr;
enum field_type rhs = sql_expr_type(e);
- zRet[i] = sql_field_type_to_affinity(sql_type_result(rhs, lhs));
+ zRet[i] = sql_type_result(rhs, lhs);
} else {
- zRet[i] = sql_field_type_to_affinity(lhs);
+ zRet[i] = lhs;
}
}
zRet[nVal] = '\0';
@@ -2826,6 +2825,7 @@ sqlite3CodeSubselect(Parse * pParse, /* Parsing context */
jmpIfDynamic = -1;
}
r3 = sqlite3ExprCodeTarget(pParse, pE2, r1);
+ assert(lhs_type < field_type_MAX);
sqlite3VdbeAddOp4(v, OP_MakeRecord, r3,
1, r2,
(char *) &lhs_type,
@@ -3142,8 +3142,7 @@ sqlite3ExprCodeIN(Parse * pParse, /* Parsing and code generating context */
* of the RHS using the LHS as a probe. If found, the result is
* true.
*/
- char *type_str = sql_affinity_str_to_field_type_str(zAff);
- sqlite3VdbeAddOp4(v, OP_ApplyType, rLhs, nVector, 0, type_str, nVector);
+ sqlite3VdbeAddOp4(v, OP_ApplyType, rLhs, nVector, 0, zAff, nVector);
if (destIfFalse == destIfNull) {
/* Combine Step 3 and Step 5 into a single opcode */
sqlite3VdbeAddOp4Int(v, OP_NotFound, pExpr->iTable,
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index acc7712f6..cc06638cb 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -41,33 +41,6 @@
#include "bit/bit.h"
#include "box/box.h"
-char *
-sql_space_index_affinity_str(struct sqlite3 *db, struct space_def *space_def,
- struct index_def *idx_def)
-{
- uint32_t column_count = idx_def->key_def->part_count;
- char *aff = (char *)sqlite3DbMallocRaw(db, column_count + 1);
- if (aff == NULL)
- return NULL;
- /*
- * Table may occasionally come from non-SQL API, so lets
- * gentle process this case by setting default affinity
- * for it.
- */
- if (space_def->fields == NULL) {
- memset(aff, AFFINITY_BLOB, column_count);
- } else {
- for (uint32_t i = 0; i < column_count; i++) {
- aff[i] = sql_space_index_part_affinity(space_def,
- idx_def, i);
- if (aff[i] == AFFINITY_UNDEFINED)
- aff[i] = AFFINITY_BLOB;
- }
- }
- aff[column_count] = '\0';
- return aff;
-}
-
char *
sql_index_type_str(struct sqlite3 *db, const struct index_def *idx_def)
{
@@ -1247,12 +1220,12 @@ xferOptimization(Parse * pParse, /* Parser context */
if (dest->def->field_count != src->def->field_count)
return 0;
for (i = 0; i < (int)dest->def->field_count; i++) {
- enum affinity_type dest_affinity =
- dest->def->fields[i].affinity;
- enum affinity_type src_affinity =
- src->def->fields[i].affinity;
- /* Affinity must be the same on all columns. */
- if (dest_affinity != src_affinity)
+ enum field_type dest_type =
+ dest->def->fields[i].type;
+ enum field_type src_type =
+ src->def->fields[i].type;
+ /* Type must be the same on all columns. */
+ if (dest_type != src_type)
return 0;
uint32_t id;
if (sql_column_collation(dest->def, i, &id) !=
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index cc3e2f2fd..f3008094b 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -1202,12 +1202,9 @@ selectInnerLoop(Parse * pParse, /* The parser context */
int r1 = sqlite3GetTempReg(pParse);
assert(sqlite3Strlen30(pDest->zAffSdst) ==
(unsigned int)nResultCol);
- char *type_str =
- sql_affinity_str_to_field_type_str(
- pDest->zAffSdst);
sqlite3VdbeAddOp4(v, OP_MakeRecord, regResult,
- nResultCol, r1, type_str,
- P4_DYNAMIC);
+ nResultCol, r1,
+ pDest->zAffSdst, nResultCol);
sqlite3ExprCacheAffinityChange(pParse,
regResult,
nResultCol);
@@ -1629,10 +1626,8 @@ generateSortTail(Parse * pParse, /* Parsing context */
assert((unsigned int)nColumn ==
sqlite3Strlen30(pDest->zAffSdst));
- const char *type_str =
- sql_affinity_str_to_field_type_str(pDest->zAffSdst);
sqlite3VdbeAddOp4(v, OP_MakeRecord, regRow, nColumn,
- regTupleid, type_str, P4_DYNAMIC);
+ regTupleid, pDest->zAffSdst, nColumn);
sqlite3ExprCacheAffinityChange(pParse, regRow, nColumn);
sqlite3VdbeAddOp2(v, OP_IdxInsert, regTupleid, pDest->reg_eph);
break;
@@ -1974,7 +1969,6 @@ sqlite3SelectAddColumnTypeAndCollation(Parse * pParse, /* Parsing contexts */
enum field_type type = sql_expr_type(p);
if (type == FIELD_TYPE_ANY)
type = FIELD_TYPE_SCALAR;
- pTab->def->fields[i].affinity = sql_field_type_to_affinity(type);
pTab->def->fields[i].type = type;
bool is_found;
uint32_t coll_id;
@@ -3067,10 +3061,9 @@ generateOutputSubroutine(struct Parse *parse, struct Select *p,
int r1;
testcase(in->nSdst > 1);
r1 = sqlite3GetTempReg(parse);
- const char *type_str =
- sql_affinity_str_to_field_type_str(dest->zAffSdst);
sqlite3VdbeAddOp4(v, OP_MakeRecord, in->iSdst,
- in->nSdst, r1, type_str, P4_DYNAMIC);
+ in->nSdst, r1, dest->zAffSdst,
+ in->nSdst);
sqlite3ExprCacheAffinityChange(parse, in->iSdst,
in->nSdst);
sqlite3VdbeAddOp2(v, OP_IdxInsert, r1, dest->reg_eph);
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 7d1159345..807ca16c6 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -3431,19 +3431,6 @@ sql_create_view(struct Parse *parse_context, struct Token *begin,
struct Token *name, struct ExprList *aliases,
struct Select *select, bool if_exists);
-/**
- * Helper to convert SQLite affinity to corresponding
- * Tarantool field type.
- **/
-enum field_type
-sql_affinity_to_field_type(enum affinity_type affinity);
-
-enum affinity_type
-sql_field_type_to_affinity(enum field_type field_type);
-
-char *
-sql_affinity_str_to_field_type_str(const char *affinity_str);
-
/**
* Compile view, i.e. create struct Select from
* 'CREATE VIEW...' string, and assign cursors to each table from
@@ -4215,33 +4202,6 @@ int sqlite3VarintLen(u64 v);
#define getVarint sqlite3GetVarint
#define putVarint sqlite3PutVarint
-/**
- * Return a pointer to the column affinity string associated with
- * given index. A column affinity string has one character for
- * each column in the table, according to the affinity of the
- * column:
- *
- * Character Column affinity
- * ------------------------------
- * 'A' BLOB
- * 'B' TEXT
- * 'C' NUMERIC
- * 'D' INTEGER
- * 'F' REAL
- *
- * Memory for the buffer containing the column index affinity string
- * is allocated on heap.
- *
- * @param db Database handle.
- * @param space_def Definition of space index belongs to.
- * @param idx_def Definition of index from which affinity
- * to be extracted.
- * @retval Allocated affinity string, or NULL on OOM.
- */
-char *
-sql_space_index_affinity_str(struct sqlite3 *db, struct space_def *space_def,
- struct index_def *idx_def);
-
/** Return string consisting of fields types of given index. */
char *
sql_index_type_str(struct sqlite3 *db, const struct index_def *idx_def);
@@ -4631,19 +4591,6 @@ int
sql_stat4_column(struct sqlite3 *db, const char *record, uint32_t col_num,
sqlite3_value **res);
-/**
- * Return the affinity for a single column of an index.
- *
- * @param def Definition of space @idx belongs to.
- * @param idx Index to be investigated.
- * @param partno Affinity of this part to be returned.
- *
- * @retval Affinity of @partno index part.
- */
-enum affinity_type
-sql_space_index_part_affinity(struct space_def *def, struct index_def *idx,
- uint32_t partno);
-
/*
* The interface to the LEMON-generated parser
*/
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 369fb4b79..24cc08e8f 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -2165,7 +2165,7 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */
} else {
/* Neither operand is NULL. Do a comparison. */
affinity = pOp->p5 & AFFINITY_MASK;
- if (affinity>=AFFINITY_INTEGER) {
+ if (sql_type_is_numeric(affinity)) {
if ((flags1 | flags3)&MEM_Str) {
if ((flags1 & (MEM_Int|MEM_Real|MEM_Str))==MEM_Str) {
applyNumericAffinity(pIn1,0);
@@ -2193,7 +2193,7 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */
res = 0;
goto compare_op;
}
- } else if (affinity==AFFINITY_TEXT) {
+ } else if (affinity == FIELD_TYPE_STRING) {
if ((flags1 & MEM_Str)==0 && (flags1 & (MEM_Int|MEM_Real))!=0) {
testcase( pIn1->flags & MEM_Int);
testcase( pIn1->flags & MEM_Real);
diff --git a/src/box/sql/where.c b/src/box/sql/where.c
index ac5390f92..88c45aaa3 100644
--- a/src/box/sql/where.c
+++ b/src/box/sql/where.c
@@ -701,7 +701,6 @@ termCanDriveIndex(WhereTerm * pTerm, /* WHERE clause term to check */
return 0;
if (pTerm->u.leftColumn < 0)
return 0;
- aff = pSrc->pTab->aCol[pTerm->u.leftColumn].affinity;
if (!sqlite3IndexAffinityOk(pTerm->pExpr, aff))
return 0;
return 1;
@@ -1139,15 +1138,6 @@ whereRangeAdjust(WhereTerm * pTerm, LogEst nNew)
return nRet;
}
-enum affinity_type
-sql_space_index_part_affinity(struct space_def *def, struct index_def *idx,
- uint32_t partno)
-{
- assert(partno < idx->key_def->part_count);
- uint32_t fieldno = idx->key_def->parts[partno].fieldno;
- return def->fields[fieldno].affinity;
-}
-
/*
* This function is called to estimate the number of rows visited by a
* range-scan on a skip-scan index. For example:
diff --git a/src/box/sql/wherecode.c b/src/box/sql/wherecode.c
index 06335bcf7..759214b22 100644
--- a/src/box/sql/wherecode.c
+++ b/src/box/sql/wherecode.c
@@ -387,19 +387,18 @@ codeApplyAffinity(Parse * pParse, int base, int n, char *zAff)
/* Adjust base and n to skip over AFFINITY_BLOB entries at the beginning
* and end of the affinity string.
*/
- while (n > 0 && zAff[0] == AFFINITY_BLOB) {
+ while (n > 0 && zAff[0] == FIELD_TYPE_SCALAR) {
n--;
base++;
zAff++;
}
- while (n > 1 && zAff[n - 1] == AFFINITY_BLOB) {
+ while (n > 1 && zAff[n - 1] == FIELD_TYPE_SCALAR) {
n--;
}
if (n > 0) {
- const char *type_str = sql_affinity_str_to_field_type_str(zAff);
- sqlite3VdbeAddOp4(v, OP_ApplyType, base, n, 0, type_str, n);
+ sqlite3VdbeAddOp4(v, OP_ApplyType, base, n, 0, zAff, n);
sqlite3ExprCacheAffinityChange(pParse, base, n);
}
}
@@ -421,12 +420,11 @@ updateRangeAffinityStr(Expr * pRight, /* RHS of comparison */
{
int i;
for (i = 0; i < n; i++) {
- enum field_type type = sql_affinity_to_field_type(zAff[i]);
Expr *p = sqlite3VectorFieldSubexpr(pRight, i);
enum field_type expr_type = sql_expr_type(p);
- if (sql_type_result(expr_type, type) == FIELD_TYPE_SCALAR ||
- sql_expr_needs_type_change(p, type)) {
- zAff[i] = AFFINITY_BLOB;
+ if (sql_type_result(expr_type, zAff[i]) == FIELD_TYPE_SCALAR ||
+ sql_expr_needs_type_change(p, zAff[i])) {
+ zAff[i] = FIELD_TYPE_SCALAR;
}
}
}
@@ -709,11 +707,7 @@ codeAllEqualityTerms(Parse * pParse, /* Parsing context */
nReg = pLoop->nEq + nExtraReg;
pParse->nMem += nReg;
-
- struct space *space = space_by_id(idx_def->space_id);
- assert(space != NULL);
- char *zAff = sql_space_index_affinity_str(pParse->db, space->def,
- idx_def);
+ char *zAff = sql_index_type_str(pParse->db, idx_def);
assert(zAff != 0 || pParse->db->mallocFailed);
if (nSkip) {
@@ -767,7 +761,7 @@ codeAllEqualityTerms(Parse * pParse, /* Parsing context */
* affinity of the comparison has been applied to the value.
*/
if (zAff)
- zAff[j] = AFFINITY_BLOB;
+ zAff[j] = FIELD_TYPE_SCALAR;
}
} else if ((pTerm->eOperator & WO_ISNULL) == 0) {
Expr *pRight = pTerm->pExpr->pRight;
@@ -779,14 +773,13 @@ codeAllEqualityTerms(Parse * pParse, /* Parsing context */
if (zAff) {
enum field_type type =
sql_expr_type(pRight);
- enum field_type idx_type =
- sql_affinity_to_field_type(zAff[j]);
+ enum field_type idx_type = zAff[j];
if (sql_type_result(type, idx_type) ==
FIELD_TYPE_SCALAR) {
- zAff[j] = AFFINITY_BLOB;
+ zAff[j] = FIELD_TYPE_SCALAR;
}
if (sql_expr_needs_type_change(pRight, idx_type))
- zAff[j] = AFFINITY_BLOB;
+ zAff[j] = FIELD_TYPE_SCALAR;
}
}
}
diff --git a/src/box/sql/whereexpr.c b/src/box/sql/whereexpr.c
index 6ffd5ae84..96838f4b2 100644
--- a/src/box/sql/whereexpr.c
+++ b/src/box/sql/whereexpr.c
@@ -284,7 +284,8 @@ like_optimization_is_valid(Parse *pParse, Expr *pExpr, Expr **ppPrefix,
Vdbe *pReprepare = pParse->pReprepare;
int iCol = pRight->iColumn;
pVal =
- sqlite3VdbeGetBoundValue(pReprepare, iCol, AFFINITY_BLOB);
+ sqlite3VdbeGetBoundValue(pReprepare, iCol,
+ FIELD_TYPE_SCALAR);
if (pVal && sqlite3_value_type(pVal) == SQLITE_TEXT) {
z = (char *)sqlite3_value_text(pVal);
}
diff --git a/test/sql/types.result b/test/sql/types.result
index 915a6341a..df4dc151e 100644
--- a/test/sql/types.result
+++ b/test/sql/types.result
@@ -33,20 +33,19 @@ box.sql.execute("CREATE TABLE t1 (id TEXT PRIMARY KEY, a REAL, b INT, c TEXT, d
...
box.space.T1:format()
---
-- [{'affinity': 66, 'type': 'string', 'nullable_action': 'abort', 'name': 'ID', 'is_nullable': false},
- {'affinity': 69, 'type': 'number', 'nullable_action': 'none', 'name': 'A', 'is_nullable': true},
- {'affinity': 68, 'type': 'integer', 'nullable_action': 'none', 'name': 'B', 'is_nullable': true},
- {'affinity': 66, 'type': 'string', 'nullable_action': 'none', 'name': 'C', 'is_nullable': true},
- {'affinity': 65, 'type': 'scalar', 'nullable_action': 'none', 'name': 'D', 'is_nullable': true}]
+- [{'type': 'string', 'nullable_action': 'abort', 'name': 'ID', 'is_nullable': false},
+ {'type': 'number', 'nullable_action': 'none', 'name': 'A', 'is_nullable': true},
+ {'type': 'integer', 'nullable_action': 'none', 'name': 'B', 'is_nullable': true},
+ {'type': 'string', 'nullable_action': 'none', 'name': 'C', 'is_nullable': true},
+ {'type': 'scalar', 'nullable_action': 'none', 'name': 'D', 'is_nullable': true}]
...
box.sql.execute("CREATE VIEW v1 AS SELECT b + a, b - a FROM t1;")
---
...
box.space.V1:format()
---
-- [{'affinity': 69, 'type': 'number', 'nullable_action': 'none', 'name': 'b + a',
- 'is_nullable': true}, {'affinity': 69, 'type': 'number', 'nullable_action': 'none',
- 'name': 'b - a', 'is_nullable': true}]
+- [{'type': 'number', 'nullable_action': 'none', 'name': 'b + a', 'is_nullable': true},
+ {'type': 'number', 'nullable_action': 'none', 'name': 'b - a', 'is_nullable': true}]
...
-- gh-2494: index's part also features correct declared type.
--
diff --git a/test/sql/upgrade.result b/test/sql/upgrade.result
index 79c7eb245..02ab9b42b 100644
--- a/test/sql/upgrade.result
+++ b/test/sql/upgrade.result
@@ -80,12 +80,12 @@ box.sql.execute("CREATE TRIGGER t2t AFTER INSERT ON t BEGIN INSERT INTO t_out VA
...
box.space._space.index['name']:get('T')
---
-- [513, 1, 'T', 'memtx', 1, {}, [{'affinity': 68, 'type': 'integer', 'nullable_action': 'abort',
- 'name': 'X', 'is_nullable': false}]]
+- [513, 1, 'T', 'memtx', 1, {}, [{'type': 'integer', 'nullable_action': 'abort', 'name': 'X',
+ 'is_nullable': false}]]
...
box.space._space.index['name']:get('T_OUT')
---
-- [514, 1, 'T_OUT', 'memtx', 1, {}, [{'affinity': 68, 'type': 'integer', 'nullable_action': 'abort',
+- [514, 1, 'T_OUT', 'memtx', 1, {}, [{'type': 'integer', 'nullable_action': 'abort',
'name': 'X', 'is_nullable': false}]]
...
t1t = box.space._trigger:get('T1T')
--
2.15.1
More information about the Tarantool-patches
mailing list