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 9FDC72310D for ; Tue, 2 Jul 2019 11:55:12 -0400 (EDT) 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 GyDLz1CksBWW for ; Tue, 2 Jul 2019 11:55:12 -0400 (EDT) Received: from smtp59.i.mail.ru (smtp59.i.mail.ru [217.69.128.39]) (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 CD6372310B for ; Tue, 2 Jul 2019 11:55:11 -0400 (EDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.8\)) Subject: [tarantool-patches] Re: [PATCH 2/2 v2] sql: allow only for string-like args From: "n.pettik" In-Reply-To: Date: Tue, 2 Jul 2019 18:55:06 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <2551430B-3DD4-4DAC-BC2B-7FA4F142A0FB@tarantool.org> References: <91edd7e8b72a93c5b5c0592c181576fca98e66fd.1561372731.git.roman.habibov@tarantool.org> <23ABB827-11CA-4A4E-AB5C-839BCA8F3A50@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: Roman Khabibov >>> @@ -4396,6 +4433,8 @@ sqlExprCodeAtInit(Parse * pParse, /* = Parsing context */ >>> int >>> sqlExprCodeTemp(Parse * pParse, Expr * pExpr, int *pReg) >>> { >>> + if (pExpr->op =3D=3D TK_COLLATE && check_collate_arg(pParse, = pExpr) !=3D 0) >>> + return 0; >>=20 >> Why this check is required (in addition to the same check in = ExprCodeTarget)? > It is needed e.g. for the following queries: =E2=80=9CSELECT NOT TRUE = COLLATE =E2=80=9Cunicode=E2=80=9D=E2=80=9D. > =E2=80=9CNOT=E2=80=9D will be on the root. The sequence of calls: = sqlExprCodeTagret -> (case TK_NOT) > -> sqlExprCodeTemp. > NOT > / \ > unicode 0x0 > / \ > TRUE 0x0 >>> int r2; >>> pExpr =3D sqlExprSkipCollate(pExpr); In fact, problem is here: this function skips collate token. So, diff is following: @@ -4427,10 +4426,7 @@ sqlExprCodeAtInit(Parse * pParse, /* = Parsing context */ int sqlExprCodeTemp(Parse * pParse, Expr * pExpr, int *pReg) { - if (pExpr->op =3D=3D TK_COLLATE && check_collate_arg(pParse, = pExpr) !=3D 0) - return 0; int r2; - pExpr =3D sqlExprSkipCollate(pExpr); if (ConstFactorOk(pParse) && pExpr->op !=3D TK_REGISTER && = sqlExprIsConstantNotJoin(pExpr) ) { What is more, you don=E2=80=99t need to set type of expression in sqlExprCodeTarget(): @@ -4201,7 +4201,6 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, = int target) } case TK_SPAN: case TK_COLLATE:{ - pExpr->pLeft->type =3D = sql_expr_type(pExpr->pLeft); if (check_collate_arg(pParse, pExpr) !=3D 0) break; return sqlExprCodeTarget(pParse, pExpr->pLeft, >>> diff --git a/test/sql-tap/in3.test.lua b/test/sql-tap/in3.test.lua >>> index 1ca6a6446..1fdee16b7 100755 >>> --- a/test/sql-tap/in3.test.lua >>> +++ b/test/sql-tap/in3.test.lua >>> @@ -186,33 +186,6 @@ test:do_test( >>> -- >>> }) >>>=20 >>> - >>> - >>> --- The first of these queries has to use the temp-table, because = the=20 >>> --- collation sequence used for the index on "t1.a" does not match = the >>> --- collation sequence used by the "IN" comparison. The second does = not >>> --- require a temp-table, because the collation sequences match. >>> --- >>> -test:do_test( >>> - "in3-1.14", >>> - function() >>> - return exec_neph(" SELECT a FROM t1 WHERE a COLLATE = \"unicode_ci\" IN (SELECT a FROM t1) ") >>> - end, { >>> - -- >>> - 1, 1, 3, 5 >>> - -- >>> - }) >>> - >>> -test:do_test( >>> - "in3-1.15", >>> - function() >>> - return exec_neph(" SELECT a FROM t1 WHERE a COLLATE = \"binary\" IN (SELECT a FROM t1) ") >>> - end, { >>> - -- >>> - 1, 1, 3, 5 >>> - -- >>> - }) >>=20 >> I=E2=80=99d better fix these tests rather than delete. >=20 > Previous test in this file will be same if I remove COLLATE clauses. You don=E2=80=99t have to remove collate clause, you should create new table with string field type and run this test using that table. Comment above the test clearly indicates its purpose.