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 8ED0831CF5 for ; Tue, 25 Jun 2019 13:30:38 -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 ML7X_YnWZQuj for ; Tue, 25 Jun 2019 13:30:38 -0400 (EDT) Received: from smtp60.i.mail.ru (smtp60.i.mail.ru [217.69.128.40]) (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 4C69831CEF for ; Tue, 25 Jun 2019 13:30:38 -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: <91edd7e8b72a93c5b5c0592c181576fca98e66fd.1561372731.git.roman.habibov@tarantool.org> Date: Tue, 25 Jun 2019 20:30:36 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <23ABB827-11CA-4A4E-AB5C-839BCA8F3A50@tarantool.org> References: <91edd7e8b72a93c5b5c0592c181576fca98e66fd.1561372731.git.roman.habibov@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 > diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c > index 898d16cf3..b850250e3 100644 > --- a/src/box/sql/expr.c > +++ b/src/box/sql/expr.c > @@ -197,6 +197,40 @@ sqlExprSkipCollate(Expr * pExpr) > return pExpr; > } >=20 > +/* > + * Check that left node of @a expr with the collation in the root > + * can be used with . If it is not, leave an error > + * message in pParse. > + * > + * @param parse Parser context. > + * @param expr Expression for checking. > + * > + * @retval 0 on success. > + * @retval -1 on error. > + */ > +static int > +check_collate_arg(struct Parse *parse, struct Expr *expr) > +{ > + struct Expr *left =3D expr->pLeft; > + while (left->op =3D=3D TK_COLLATE) > + left =3D left->pLeft; > + enum field_type type; > + if (left->op =3D=3D TK_COLUMN) { > + int col_num =3D left->iColumn; > + type =3D left->space_def->fields[col_num].type; > + } else { > + type =3D left->type; > + } > + if (type !=3D FIELD_TYPE_STRING && type !=3D FIELD_TYPE_SCALAR) = { > + diag_set(ClientError, ER_SQL_PARSER_GENERIC, > + "COLLATE can't be used with " > + "non-string arguments=E2=80=9D); -> COLLATE clause ... > + parse->is_aborted =3D true; > + return -1; > + } > + return 0; > +} > + > int > sql_expr_coll(Parse *parse, Expr *p, bool *is_explicit_coll, uint32_t = *coll_id, > struct coll **coll) > @@ -4173,6 +4207,9 @@ 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, > target); > } > @@ -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; Why this check is required (in addition to the same check in = ExprCodeTarget)? > int r2; > pExpr =3D sqlExprSkipCollate(pExpr); > if (ConstFactorOk(pParse) > diff --git a/test/sql-tap/collation.test.lua = b/test/sql-tap/collation.test.lua > index f36540fc2..f614d0e86 100755 > --- a/test/sql-tap/collation.test.lua > +++ b/test/sql-tap/collation.test.lua > @@ -1,6 +1,6 @@ > #!/usr/bin/env tarantool > test =3D require("sqltester") > -test:plan(177) > +test:plan(191) >=20 > local prefix =3D "collation-" >=20 > @@ -546,4 +546,77 @@ test:do_execsql_test( > [[ SELECT s COLLATE "unicode_ci" FROM a ORDER BY s COLLATE = "unicode" ]], > {"b","B"}) >=20 > +-- gh-3805 Check COLLATE passing with string-like args only. > + > +test:do_execsql_test( > + "collation-2.7.0", > + [[ CREATE TABLE test1 (one INT PRIMARY KEY, two INT) ]], > + {}) > + > +test:do_catchsql_test( > + "collation-2.7.1", > + 'SELECT one COLLATE BINARY FROM test1=E2=80=99, BINARY is non-existent collation. Correct usage is =E2=80=9Cbinary=E2=80=9D= . > +test:do_catchsql_test( > + "collation-2.7.14", > + 'SELECT +\'str\' COLLATE \"unicode\"', > + {0,{"str"}}) What about concatenation operation? > 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 > - -- > - }) I=E2=80=99d better fix these tests rather than delete.