From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id F0EC7234B7 for ; Fri, 28 Dec 2018 04:35:02 -0500 (EST) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 618Eu83Ogahk for ; Fri, 28 Dec 2018 04:35:02 -0500 (EST) Received: from smtp40.i.mail.ru (smtp40.i.mail.ru [94.100.177.100]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 722E622B44 for ; Fri, 28 Dec 2018 04:35:02 -0500 (EST) From: Nikita Pettik Subject: [tarantool-patches] [PATCH 7/8] sql: clean-up affinity from SQL source code Date: Fri, 28 Dec 2018 11:34:51 +0200 Message-Id: In-Reply-To: References: In-Reply-To: References: Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: v.shpilevoy@tarantool.org, Nikita Pettik 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