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 741AD247A7 for ; Mon, 28 Jan 2019 11:39:59 -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 Az0bRaP3iNZP for ; Mon, 28 Jan 2019 11:39:59 -0500 (EST) Received: from smtp58.i.mail.ru (smtp58.i.mail.ru [217.69.128.38]) (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 D99B020EF7 for ; Mon, 28 Jan 2019 11:39:58 -0500 (EST) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.2 \(3445.102.3\)) Subject: [tarantool-patches] Re: [PATCH 6/8] sql: replace affinity with field type in struct Expr From: "n.pettik" In-Reply-To: <2837d5ad-fc59-8c3e-0fef-b82ea764f6ca@tarantool.org> Date: Mon, 28 Jan 2019 19:39:57 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <7FD8757A-713C-4887-9268-42849FFEE3A8@tarantool.org> References: <406a1cab-d3cd-437b-668e-141697fbd637@tarantool.org> <8D9E26B4-FC3D-4EBD-9941-5570A5ECFA17@tarantool.org> <2837d5ad-fc59-8c3e-0fef-b82ea764f6ca@tarantool.org> 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: Vladislav Shpilevoy >>>> -int >>>> -sqlite3IndexAffinityOk(Expr * pExpr, char idx_affinity) >>>> +enum field_type >>>> +sql_index_type_is_ok(struct Expr *expr, enum field_type idx_type) >>>=20 >>> 4. Strictly speaking, it is not idx_type. It is a type of one >>> column. I think, it should be renamed to just type, or field_type, >>> as well as in struct WhereScan. >> Yea, it=E2=80=99s fair. Renamed: >> -/* >> - * pExpr is a comparison operator. Return the type affinity that = should >> - * be applied to both operands prior to doing the comparison. >> +/** >> + * pExpr is a comparison operator. Return the type affinity >> + * that should be applied to both operands prior to doing >> + * the comparison. >> */ >> static enum field_type >> -comparisonAffinity(Expr * pExpr) >> +expr_cmp_mutual_type(struct Expr *pExpr) >> { >> assert(pExpr->op =3D=3D TK_EQ || pExpr->op =3D=3D TK_IN || = pExpr->op =3D=3D TK_LT || >> pExpr->op =3D=3D TK_GT || pExpr->op =3D=3D TK_GE || = pExpr->op =3D=3D TK_LE || >=20 > Are you sure? Looks like it is not renamed. Also, the comment > was about sql_index_type_is_ok and its parameter idx_type, but you > changed comparisonAffinity function. Sorry, I renamed function, but forgot about param. Fixed: diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index 1de980f00..1d4ea6ef0 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -281,21 +281,21 @@ expr_cmp_mutual_type(struct Expr *pExpr) =20 /** * @param expr is a comparison expression, eg. '=3D', '<', IN(...) etc. - * @param idx_type is the type of an indexed column. - * @retval Return true if the index with @idx_type may be used to + * @param field_type is the type of an indexed column. + * @retval Return true if the index with @field_type may be used to * implement the comparison in expr. */ enum field_type -sql_expr_cmp_type_is_compatible(struct Expr *expr, enum field_type = idx_type) +sql_expr_cmp_type_is_compatible(struct Expr *expr, enum field_type = field_type) { enum field_type type =3D expr_cmp_mutual_type(expr); switch (type) { case FIELD_TYPE_SCALAR: return 1; case FIELD_TYPE_STRING: - return idx_type =3D=3D FIELD_TYPE_STRING; + return field_type =3D=3D FIELD_TYPE_STRING; default: - return sql_type_is_numeric(idx_type); + return sql_type_is_numeric(field_type); } } >>>> @@ -2826,16 +2795,15 @@ sqlite3CodeSubselect(Parse * pParse, = /* Parsing context */ >>>> * that columns affinity when building = index keys. If is not >>>> * a column, use numeric affinity. >>>> */ >>>> - char affinity; /* Affinity of the LHS = of the IN */ >>>> int i; >>>> ExprList *pList =3D pExpr->x.pList; >>>> struct ExprList_item *pItem; >>>> int r1, r2, r3; >>>> - affinity =3D sqlite3ExprAffinity(pLeft); >>>> - if (!affinity) { >>>> - affinity =3D AFFINITY_BLOB; >>>> - } >>>> + enum field_type lhs_type =3D >>>> + sql_expr_type(pLeft); >>>> + if (lhs_type =3D=3D FIELD_TYPE_ANY) >>>> + lhs_type =3D FIELD_TYPE_SCALAR; >>>=20 >>> 7. Why did not you move this conversion to sql_expr_type? I mean >>> ANY -> SCALAR. >> Unfortunately, right now I can=E2=80=99t replace it with ANY. It is = still used >> when IN operator comes with one operand (see EP_Generic flag). >> I have already sent patch which removes it: >> = https://github.com/tarantool/tarantool/tree/np/gh-3934-IN-operator-fix >=20 > I've already acked the patch. Please, hurry up Kirill Y. to push it, > rebase this branch, and replace ANY with SCALAR. I had to add some other fixes to make this happen: Changes to sql: replace affinity with field type for func: diff --git a/src/box/sql/date.c b/src/box/sql/date.c index e452aad73..f32a5841a 100644 --- a/src/box/sql/date.c +++ b/src/box/sql/date.c @@ -1307,14 +1307,14 @@ sqlite3RegisterDateTimeFunctions(void) static FuncDef aDateTimeFuncs[] =3D { #ifndef SQLITE_OMIT_DATETIME_FUNCS DFUNCTION(julianday, -1, 0, 0, juliandayFunc, = FIELD_TYPE_NUMBER), - DFUNCTION(date, -1, 0, 0, dateFunc, FIELD_TYPE_NUMBER), - DFUNCTION(time, -1, 0, 0, timeFunc, FIELD_TYPE_NUMBER), - DFUNCTION(datetime, -1, 0, 0, datetimeFunc, = FIELD_TYPE_NUMBER), - DFUNCTION(strftime, -1, 0, 0, strftimeFunc, = FIELD_TYPE_NUMBER), - DFUNCTION(current_time, 0, 0, 0, ctimeFunc, = FIELD_TYPE_NUMBER), + DFUNCTION(date, -1, 0, 0, dateFunc, FIELD_TYPE_STRING), + DFUNCTION(time, -1, 0, 0, timeFunc, FIELD_TYPE_STRING), + DFUNCTION(datetime, -1, 0, 0, datetimeFunc, = FIELD_TYPE_STRING), + DFUNCTION(strftime, -1, 0, 0, strftimeFunc, = FIELD_TYPE_STRING), + DFUNCTION(current_time, 0, 0, 0, ctimeFunc, = FIELD_TYPE_STRING), DFUNCTION(current_timestamp, 0, 0, 0, ctimestampFunc, - FIELD_TYPE_NUMBER), - DFUNCTION(current_date, 0, 0, 0, cdateFunc, = FIELD_TYPE_NUMBER), + FIELD_TYPE_STRING), + DFUNCTION(current_date, 0, 0, 0, cdateFunc, = FIELD_TYPE_STRING), #else STR_FUNCTION(current_time, 0, "%H:%M:%S", 0, = currentTimeFunc), STR_FUNCTION(current_date, 0, "%Y-%m-%d", 0, = currentTimeFunc), diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index 347fee563..58e35e0d9 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -3995,7 +3995,7 @@ sqlite3ExprCodeTarget(Parse * pParse, Expr * = pExpr, int target) break; } =20 - if (pDef->ret_type !=3D AFFINITY_UNDEFINED) { + if (pDef->ret_type !=3D FIELD_TYPE_SCALAR) { pExpr->affinity =3D = sql_field_type_to_affinity(pDef->ret_type); } else { diff --git a/src/box/sql/func.c b/src/box/sql/func.c index 08228fbbe..147f93ae4 100644 --- a/src/box/sql/func.c +++ b/src/box/sql/func.c @@ -1733,14 +1733,14 @@ sqlite3RegisterBuiltinFunctions(void) FUNCTION(rtrim, 2, 2, 0, trimFunc, FIELD_TYPE_STRING), FUNCTION(trim, 1, 3, 0, trimFunc, FIELD_TYPE_STRING), FUNCTION(trim, 2, 3, 0, trimFunc, FIELD_TYPE_STRING), - FUNCTION(min, -1, 0, 1, minmaxFunc, 0), - FUNCTION(min, 0, 0, 1, 0, 0), + FUNCTION(min, -1, 0, 1, minmaxFunc, FIELD_TYPE_SCALAR), + FUNCTION(min, 0, 0, 1, 0, FIELD_TYPE_SCALAR), AGGREGATE2(min, 1, 0, 1, minmaxStep, minMaxFinalize, - SQLITE_FUNC_MINMAX, 0), - FUNCTION(max, -1, 1, 1, minmaxFunc, 0), - FUNCTION(max, 0, 1, 1, 0, 0), + SQLITE_FUNC_MINMAX, FIELD_TYPE_SCALAR), + FUNCTION(max, -1, 1, 1, minmaxFunc, FIELD_TYPE_SCALAR), + FUNCTION(max, 0, 1, 1, 0, FIELD_TYPE_SCALAR), AGGREGATE2(max, 1, 1, 1, minmaxStep, minMaxFinalize, - SQLITE_FUNC_MINMAX, 0), + SQLITE_FUNC_MINMAX, FIELD_TYPE_SCALAR), FUNCTION2(typeof, 1, 0, 0, typeofFunc, = SQLITE_FUNC_TYPEOF, FIELD_TYPE_STRING), FUNCTION2(length, 1, 0, 0, lengthFunc, = SQLITE_FUNC_LENGTH, @@ -1769,9 +1769,9 @@ sqlite3RegisterBuiltinFunctions(void) FUNCTION(zeroblob, 1, 0, 0, zeroblobFunc, = FIELD_TYPE_SCALAR), FUNCTION(substr, 2, 0, 0, substrFunc, = FIELD_TYPE_STRING), FUNCTION(substr, 3, 0, 0, substrFunc, = FIELD_TYPE_STRING), - AGGREGATE(sum, 1, 0, 0, sumStep, sumFinalize, 0), - AGGREGATE(total, 1, 0, 0, sumStep, totalFinalize, 0), - AGGREGATE(avg, 1, 0, 0, sumStep, avgFinalize, 0), + AGGREGATE(sum, 1, 0, 0, sumStep, sumFinalize, = FIELD_TYPE_NUMBER), + AGGREGATE(total, 1, 0, 0, sumStep, totalFinalize, = FIELD_TYPE_NUMBER), + AGGREGATE(avg, 1, 0, 0, sumStep, avgFinalize, = FIELD_TYPE_NUMBER), AGGREGATE2(count, 0, 0, 0, countStep, countFinalize, SQLITE_FUNC_COUNT, FIELD_TYPE_INTEGER), AGGREGATE(count, 1, 0, 0, countStep, countFinalize, @@ -1788,9 +1788,10 @@ sqlite3RegisterBuiltinFunctions(void) #ifdef SQLITE_ENABLE_UNKNOWN_SQL_FUNCTION FUNCTION(unknown, -1, 0, 0, unknownFunc, 0), #endif - FUNCTION(coalesce, 1, 0, 0, 0, 0), - FUNCTION(coalesce, 0, 0, 0, 0, 0), - FUNCTION2(coalesce, -1, 0, 0, noopFunc, = SQLITE_FUNC_COALESCE, 0), + FUNCTION(coalesce, 1, 0, 0, 0, FIELD_TYPE_SCALAR), + FUNCTION(coalesce, 0, 0, 0, 0, FIELD_TYPE_SCALAR), + FUNCTION2(coalesce, -1, 0, 0, noopFunc, = SQLITE_FUNC_COALESCE, + FIELD_TYPE_SCALAR), }; sqlite3AnalyzeFunctions(); sqlite3RegisterDateTimeFunctions(); diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c index 4c6baaac6..cbda48f78 100644 --- a/src/box/sql/resolve.c +++ b/src/box/sql/resolve.c @@ -637,6 +637,8 @@ resolveExprStep(Walker * pWalker, Expr * pExpr) } } else { is_agg =3D pDef->xFinalize !=3D 0; + pExpr->affinity =3D + = sql_field_type_to_affinity(pDef->ret_type); if (pDef->funcFlags & = SQLITE_FUNC_UNLIKELY) { ExprSetProperty(pExpr, EP_Unlikely | = EP_Skip); diff --git a/test-run b/test-run index 02207efd2..4c3f11812 160000 --- a/test-run +++ b/test-run @@ -1 +1 @@ -Subproject commit 02207efd2ca44067b76c79bf012f142016d929ae +Subproject commit 4c3f11812c0c17f08780f31b6a748eef1126b2e2 diff --git a/test/sql-tap/analyze9.test.lua = b/test/sql-tap/analyze9.test.lua index df62a1624..c43437712 100755 --- a/test/sql-tap/analyze9.test.lua +++ b/test/sql-tap/analyze9.test.lua @@ -58,6 +58,9 @@ msgpack_decode_sample =3D function(txt) end i =3D i+1 end + if type(decoded_str) =3D=3D "number" then + return tostring(decoded_str) + end return decoded_str end =20 @@ -332,7 +335,7 @@ test:do_execsql_test( test:do_execsql_test( 4.7, [[ - SELECT count(*) FROM "_sql_stat4" WHERE = msgpack_decode_sample("sample") IN (34, 68, 102, 136, 170, 204, 238, = 272); + SELECT count(*) FROM "_sql_stat4" WHERE = lrange(msgpack_decode_sample("sample"), 1, 1) IN ('34', '68', '102', = '136', '170', '204', '238', '272'); ]], { -- <4.7> 8 @@ -367,8 +370,8 @@ test:do_execsql_test( SELECT msgpack_decode_sample("sample") FROM "_sql_stat4"; ]], { -- <4.9> - "x", 1110, 2230, 2750, 3350, 4090, 4470, 4980, 5240, 5280, = 5290, 5590, 5920,=20 - 5930, 6220, 6710, 7000, 7710, 7830, 7970, 8890, 8950, 9240, = 9250, 9680 + "x", "1110", "2230", "2750", "3350", "4090", "4470", "4980", = "5240", "5280", "5290", "5590", "5920", + "5930", "6220", "6710", "7000", "7710", "7830", "7970", "8890", = "8950", "9240", "9250", "9680" -- }) Additional changes to sql: replace affinity with field type in struct = Expr: Firstly, I placed several asserts like assert(type !=3D FIELD_TYPE_ANY); and noted that not for all expressions its true. So I patched = sql_expr_type function: diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index fdf00273a..e1a0dfa73 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -74,10 +74,29 @@ sql_expr_type(struct Expr *pExpr) case TK_MINUS: case TK_STAR: case TK_SLASH: + case TK_REM: + case TK_BITAND: + case TK_BITOR: + case TK_LSHIFT: + case TK_RSHIFT: assert(pExpr->pRight !=3D NULL && pExpr->pLeft !=3D = NULL); enum field_type lhs_type =3D = sql_expr_type(pExpr->pLeft); enum field_type rhs_type =3D = sql_expr_type(pExpr->pRight); return sql_type_result(rhs_type, lhs_type); + case TK_CONCAT: + return FIELD_TYPE_STRING; + case TK_CASE: + assert(pExpr->x.pList->nExpr >=3D 2); + /* + * CASE expression comes at least with one + * WHEN and one THEN clauses. So, first + * expression always represents WHEN + * argument, and the second one - THEN. + * + * TODO: We must ensure that all THEN clauses + * have arguments of the same type. + */ + return sql_expr_type(pExpr->x.pList->a[1].pExpr); case TK_LT: case TK_GT: case TK_EQ: @@ -92,16 +111,14 @@ sql_expr_type(struct Expr *pExpr) * return INTEGER. */ return FIELD_TYPE_INTEGER; - } - /* - * In case of unary plus we shouldn't discard - * type of operand (since plus always features - * NUMERIC type). - */ - if (op =3D=3D TK_UPLUS) { + case TK_UMINUS: + case TK_UPLUS: + case TK_NO: + case TK_BITNOT: assert(pExpr->pRight =3D=3D NULL); - return pExpr->pLeft->type; + return sql_expr_type(pExpr->pLeft); } + return pExpr->type; } Then, I make default type of expression be SCALAR except for binding params: diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y index ef691cb5c..d6fc41e3b 100644 --- a/src/box/sql/parse.y +++ b/src/box/sql/parse.y @@ -829,6 +829,18 @@ idlist(A) ::=3D nm(Y). case TK_FLOAT: p->type =3D FIELD_TYPE_NUMBER; break; + case TK_VARIABLE: + /* + * For variables we set BOOLEAN type since + * unassigned bindings will be replaced + * with NULL automatically, i.e. without + * explicit call of sql_bind_value(). + */ + p->type =3D FIELD_TYPE_BOOLEAN; + break; + default: + p->type =3D FIELD_TYPE_SCALAR; + break; } p->op =3D (u8)op; p->flags =3D EP_Leaf; diff --git a/src/box/sql/select.c b/src/box/sql/select.c index 8a8ccc68c..2cc0a4a78 100644 --- a/src/box/sql/select.c +++ b/src/box/sql/select.c @@ -1725,6 +1726,8 @@ generateColumnNames(Parse * pParse, /* = Parser context */ p =3D pEList->a[i].pExpr; if (NEVER(p =3D=3D 0)) continue; + if (p->op =3D=3D TK_VARIABLE) + var_pos[var_count++] =3D i; switch (p->type) { case FIELD_TYPE_INTEGER: sqlite3VdbeSetColName(v, i, COLNAME_DECLTYPE, = "INTEGER", @@ -1742,21 +1745,12 @@ generateColumnNames(Parse * pParse, /* = Parser context */ sqlite3VdbeSetColName(v, i, COLNAME_DECLTYPE, = "BLOB", SQLITE_TRANSIENT); break; - default: ; - char *type; - /* - * For variables we set BOOLEAN type since - * unassigned bindings will be replaced - * with NULL automatically, i.e. without - * explicit call of sql_bind_value(). - */ - if (p->op =3D=3D TK_VARIABLE) { - var_pos[var_count++] =3D i; - type =3D "BOOLEAN"; - } else { - type =3D "UNKNOWN"; - } - sqlite3VdbeSetColName(v, i, COLNAME_DECLTYPE, = type, + case FIELD_TYPE_BOOLEAN: + sqlite3VdbeSetColName(v, i, COLNAME_DECLTYPE, = "BOOLEAN", + SQLITE_TRANSIENT); + break; + default: + sqlite3VdbeSetColName(v, i, COLNAME_DECLTYPE, = "UNKNOWN", SQLITE_TRANSIENT); } if (pEList->a[i].zName) { Finally, I simplified derived type calculations and removed all ANY -> SCALAR transformations: diff --git a/src/box/sql/select.c b/src/box/sql/select.c index 8a8ccc68c..2cc0a4a78 100644 --- a/src/box/sql/select.c +++ b/src/box/sql/select.c @@ -1973,8 +1967,6 @@ sqlite3SelectAddColumnTypeAndCollation(Parse * = pParse, /* Parsing contexts */ for (uint32_t i =3D 0; i < pTab->def->field_count; i++) { p =3D a[i].pExpr; enum field_type type =3D sql_expr_type(p); - if (type =3D=3D FIELD_TYPE_ANY) - type =3D FIELD_TYPE_SCALAR; pTab->def->fields[i].affinity =3D = sql_field_type_to_affinity(type); pTab->def->fields[i].type =3D type; bool is_found; diff --git a/src/box/sql/wherecode.c b/src/box/sql/wherecode.c index 930b53b2b..44305f7dc 100644 --- a/src/box/sql/wherecode.c +++ b/src/box/sql/wherecode.c @@ -1169,7 +1169,7 @@ sqlite3WhereCodeOneLoopStart(WhereInfo * pWInfo, = /* Complete information about t int fieldno =3D idx_pk->key_def->parts[0].fieldno; enum field_type fd_type =3D is_format_set ? = space->def->fields[fieldno].type : - FIELD_TYPE_ANY; + FIELD_TYPE_SCALAR; =20 uint32_t pk_part_count =3D idx_pk->key_def->part_count; if (pk_part_count =3D=3D 1 && fd_type =3D=3D = FIELD_TYPE_INTEGER) { diff --git a/src/box/sql/whereexpr.c b/src/box/sql/whereexpr.c index 6ffd5ae84..1fb5fa593 100644 --- a/src/box/sql/whereexpr.c +++ b/src/box/sql/whereexpr.c @@ -758,7 +758,7 @@ exprAnalyzeOrTerm(SrcList * pSrc, /* the FROM = clause */ enum field_type lhs =3D = sql_expr_type(pOrTerm->pExpr-> pLeft); - if (rhs !=3D FIELD_TYPE_ANY && + if (rhs !=3D FIELD_TYPE_SCALAR = && rhs !=3D lhs) { okToChngToIN =3D 0; } else { diff --git a/src/box/sql.c b/src/box/sql.c index 530ec2384..cb8f6a60a 100644 --- a/src/box/sql.c +++ b/src/box/sql.c @@ -387,8 +387,8 @@ sql_ephemeral_space_create(uint32_t field_count, = struct sql_key_info *key_info) part->sort_order =3D SORT_ORDER_ASC; if (def !=3D NULL && i < def->part_count) { assert(def->parts[i].type < field_type_MAX); - part->type =3D def->parts[i].type !=3D = FIELD_TYPE_ANY ? - def->parts[i].type : = FIELD_TYPE_SCALAR; + assert(def->parts[i].type !=3D FIELD_TYPE_ANY); + part->type =3D def->parts[i].type; part->coll_id =3D def->parts[i].coll_id; } else { part->coll_id =3D COLL_NONE; diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index fdf00273a..e1a0dfa73 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -231,22 +248,9 @@ sql_expr_coll(Parse *parse, Expr *p, bool = *is_explicit_coll, uint32_t *coll_id) enum field_type sql_type_result(enum field_type lhs, enum field_type rhs) { - if (lhs !=3D FIELD_TYPE_ANY && rhs !=3D FIELD_TYPE_ANY) { - if (sql_type_is_numeric(lhs) || = sql_type_is_numeric(rhs)) - return FIELD_TYPE_NUMBER; - else - return FIELD_TYPE_SCALAR; - } else if (lhs =3D=3D FIELD_TYPE_ANY && rhs =3D=3D = FIELD_TYPE_ANY) { - /* - * Neither side of the comparison is a column. - * Compare the results directly. - */ - return FIELD_TYPE_SCALAR; - } else { - if (lhs =3D=3D FIELD_TYPE_ANY) - return rhs; - return lhs; - } + if (sql_type_is_numeric(lhs) || sql_type_is_numeric(rhs)) + return FIELD_TYPE_NUMBER; + return FIELD_TYPE_SCALAR; } @@ -2784,8 +2788,6 @@ sqlite3CodeSubselect(Parse * pParse, /* = Parsing context */ =20 enum field_type lhs_type =3D sql_expr_type(pLeft); - if (lhs_type =3D=3D FIELD_TYPE_ANY) - lhs_type =3D FIELD_TYPE_SCALAR; bool unused; sql_expr_coll(pParse, pExpr->pLeft, &unused, = &key_info->parts[0].coll_id); Test below are changed due to correct calculation of derived type for unary operators: you may notice that before it was simple expr->type. However, it didn=E2=80=99t give us original type of expr: for column, for instance we should fetch it from space_def. diff --git a/test/sql-tap/where2.test.lua b/test/sql-tap/where2.test.lua index 9089c97f9..56bec153f 100755 --- a/test/sql-tap/where2.test.lua +++ b/test/sql-tap/where2.test.lua @@ -653,14 +653,13 @@ test:do_test( "where2-6.9", function() return queryplan([[ - -- The + operator removes affinity from the rhs. No conversions - -- occur and the comparison is false. The result is an empty set. + -- The + operator doesn't affect RHS. -- SELECT b,a FROM t2249b CROSS JOIN t2249a WHERE a=3D+b; ]]) end, { -- - "nosort", "T2249B", "*", "T2249A", "*" + 123, "0123", "nosort", "T2249B", "*", "T2249A", "*" -- }) =20 @@ -673,7 +672,7 @@ test:do_test( ]]) end, { -- - "nosort", "T2249B", "*", "T2249A", "*" + 123, "0123","nosort", "T2249B", "*", "T2249A", "*" -- }) =20 @@ -681,9 +680,6 @@ test:do_test( "where2-6.10", function() return queryplan([[ - -- Use + on both sides of the comparison to disable indices - -- completely. Make sure we get the same result. - -- SELECT b,a FROM t2249b CROSS JOIN t2249a WHERE +a=3D+b; ]]) end, { @@ -753,45 +749,36 @@ test:do_test( test:do_test( "where2-6.12", function() - -- In this case, the +b disables the affinity conflict and = allows - -- the OR optimization to be used again. The result is now = an empty - -- set, the same as in where2-6.9. return queryplan([[ SELECT b,a FROM t2249b CROSS JOIN t2249a WHERE a=3D+b OR = a=3D'hello'; ]]) end, { -- - "nosort", "T2249B", "*", "T2249A", "*" + 123, "0123", "nosort", "T2249B", "*", "T2249A", "*" -- }) =20 test:do_test( "where2-6.12.2", function() - -- In this case, the +b disables the affinity conflict and = allows - -- the OR optimization to be used again. The result is now = an empty - -- set, the same as in where2-6.9. return queryplan([[ SELECT b,a FROM t2249b CROSS JOIN t2249a WHERE a=3D'hello' OR = +b=3Da; ]]) end, { -- - "nosort", "T2249B", "*", "T2249A", "*" + 123, "0123", "nosort", "T2249B", "*", "T2249A", "*" -- }) =20 test:do_test( "where2-6.12.3", function() - -- In this case, the +b disables the affinity conflict and = allows - -- the OR optimization to be used again. The result is now = an empty - -- set, the same as in where2-6.9. return queryplan([[ SELECT b,a FROM t2249b CROSS JOIN t2249a WHERE +b=3Da OR = a=3D'hello'; ]]) end, { -- - "nosort", "T2249B", "*", "T2249A", "*" + 123, "0123", "nosort", "T2249B", "*", "T2249A", "*" -- }) >=20 >> After it hits 2.1, I will replace SCALAR with ANY. >=20 > Vice versa? Not SCALAR with ANY, but ANY with SCALAR, I think. Yep. >>> ANY differs from SCALAR in only one thing - it is >>> able to store MP_MAP and MP_ARRAY. So I am slightly bent upon >>> frequency of ANY usage in SQL, wherein MAP/ARRAY does not exist. >> But still we can SELECT data from spaces created from Lua. >> For instance, _fk_constraint features type ARRAY in its format, >> so we can=E2=80=99t ignore this type even now (IMHO). >=20 > Despite our ability to select complex types, in SQL they are all > mere binary data with *sub*type msgpack, not array/map type. So > in SQL we never truly work with anything but SCALAR. Wait, at the stage of query compiling we don=E2=80=99t operate on any subtypes. We simply fetch space_def->fields->type, which may turn out to be FIELD_TYPE_MAP/ARRAY or whatever. >> -/* >> - * Return TRUE if the given expression is a constant which would be >> - * unchanged by OP_ApplyType with the type given in the second >> - * argument. >> - * >> - * This routine is used to determine if the OP_ApplyType operation >> - * can be omitted. When in doubt return FALSE. A false negative >> - * is harmless. A false positive, however, can result in the wrong >> - * answer. >> - */ >> int >> -sqlite3ExprNeedsNoAffinityChange(const Expr * p, char aff) >> +sql_expr_needs_no_type_change(const Expr *p, enum field_type type) >=20 > 1. const Expr -> const struct Expr Fixed: diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index 1de980f00..ddce8c13c 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -2117,7 +2117,7 @@ sqlite3ExprCanBeNull(const Expr * p) } =20 int -sql_expr_needs_no_type_change(const Expr *p, enum field_type type) +sql_expr_needs_no_type_change(const struct Expr *p, enum field_type = type) { u8 op; if (type =3D=3D FIELD_TYPE_SCALAR) diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h index f44235cb6..fac650f62 100644 --- a/src/box/sql/sqliteInt.h +++ b/src/box/sql/sqliteInt.h @@ -3721,7 +3721,7 @@ int sqlite3ExprCanBeNull(const Expr *); * answer. */ int -sql_expr_needs_no_type_change(const Expr *epr, enum field_type type); +sql_expr_needs_no_type_change(const struct Expr *expr, enum field_type = type); >> +/** >> + * Return TRUE if the given expression is a constant which would >> + * be unchanged by OP_ApplyType with the type given in the second >> + * argument. >> + * >> + * This routine is used to determine if the OP_ApplyType operation >> + * can be omitted. When in doubt return FALSE. A false negative >> + * is harmless. A false positive, however, can result in the wrong >> + * answer. >> + */ >> +int >> +sql_expr_needs_no_type_change(const Expr *epr, enum field_type = type); >=20 > 2. epr -> p, and the same as 1. Fixed, see above. >> diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c >> index cd71641b0..d12a2a833 100644 >> --- a/src/box/sql/vdbemem.c >> +++ b/src/box/sql/vdbemem.c >> @@ -1316,8 +1315,8 @@ valueFromExpr(sqlite3 * db, /* The database = connection */ >> sqlite3ValueSetStr(pVal, -1, zVal, = SQLITE_DYNAMIC); >> } >> if ((op =3D=3D TK_INTEGER || op =3D=3D TK_FLOAT) >> - && affinity =3D=3D AFFINITY_BLOB) { >> - sqlite3ValueApplyAffinity(pVal, AFFINITY_REAL); >> + && affinity =3D=3D FIELD_TYPE_SCALAR) { >> + sqlite3ValueApplyAffinity(pVal, = FIELD_TYPE_INTEGER); >=20 > 3. Why before your patch it was REAL and now it is INTEGER? Typo. Fixed in previous patch (while I was renaming affinity vars to = type). >> diff --git a/src/box/sql/wherecode.c b/src/box/sql/wherecode.c >> index efbc91cf8..33b860f36 100644 >> --- a/src/box/sql/wherecode.c >> +++ b/src/box/sql/wherecode.c >> @@ -423,10 +423,11 @@ updateRangeAffinityStr(Expr * pRight, /* RHS = of comparison */ >> { >> int i; >> for (i =3D 0; i < n; i++) { >> + enum field_type type =3D = sql_affinity_to_field_type(zAff[i]); >> Expr *p =3D sqlite3VectorFieldSubexpr(pRight, i); >> - enum affinity_type aff =3D sqlite3ExprAffinity(p); >> - if (sql_affinity_result(aff, zAff[i]) =3D=3D = AFFINITY_BLOB >> - || sqlite3ExprNeedsNoAffinityChange(p, zAff[i])) { >> + enum field_type expr_type =3D sql_expr_type(p); >> + if (sql_type_result(expr_type, type) =3D=3D = FIELD_TYPE_SCALAR || >> + sql_expr_needs_no_type_change(p, type)) { >> zAff[i] =3D AFFINITY_BLOB; >> } >> } >=20 > 4. Something is wrong with formatting. sql_expr_needs_no_type_change = should be > aligned under sql_type_result as a part of the condition. Fixed: diff --git a/src/box/sql/wherecode.c b/src/box/sql/wherecode.c index 33b860f36..bc57efe18 100644 --- a/src/box/sql/wherecode.c +++ b/src/box/sql/wherecode.c @@ -427,7 +427,7 @@ updateRangeAffinityStr(Expr * pRight, /* RHS = of comparison */ Expr *p =3D sqlite3VectorFieldSubexpr(pRight, i); enum field_type expr_type =3D sql_expr_type(p); if (sql_type_result(expr_type, type) =3D=3D = FIELD_TYPE_SCALAR || - sql_expr_needs_no_type_change(p, type)) { + sql_expr_needs_no_type_change(p, type)) { zAff[i] =3D AFFINITY_BLOB; } } >> @@ -1166,20 +1167,12 @@ sqlite3WhereCodeOneLoopStart(WhereInfo * = pWInfo, /* Complete information about t >> } >> struct index_def *idx_pk =3D space->index[0]->def; >> int fieldno =3D idx_pk->key_def->parts[0].fieldno; >> - char affinity =3D is_format_set ? >> - space->def->fields[fieldno].affinity : >> - AFFINITY_BLOB; >> - if (affinity =3D=3D AFFINITY_UNDEFINED) { >> - if (idx_pk->key_def->part_count =3D=3D 1 && >> - space->def->fields[fieldno].type =3D=3D >> - FIELD_TYPE_INTEGER) >> - affinity =3D AFFINITY_INTEGER; >> - else >> - affinity =3D AFFINITY_BLOB; >> - } >> + char fd_type =3D is_format_set ? >> + space->def->fields[fieldno].type : >> + FIELD_TYPE_ANY; >>=20 >=20 > 5. Why char? And the alignment is slightly wrong. Non-first lines > should be aligned under is_format_set. Fixed: @@ -1167,9 +1167,9 @@ sqlite3WhereCodeOneLoopStart(WhereInfo * pWInfo, = /* Complete information about t } struct index_def *idx_pk =3D space->index[0]->def; int fieldno =3D idx_pk->key_def->parts[0].fieldno; - char fd_type =3D is_format_set ? - space->def->fields[fieldno].type : - FIELD_TYPE_ANY; + enum field_type fd_type =3D is_format_set ? + = space->def->fields[fieldno].type : + FIELD_TYPE_SCALAR;