From: "n.pettik" <korablev@tarantool.org> To: tarantool-patches@freelists.org Cc: Roman Khabibov <roman.habibov@tarantool.org> Subject: [tarantool-patches] Re: [PATCH 2/2 v2] sql: allow <COLLATE> only for string-like args Date: Tue, 25 Jun 2019 20:30:36 +0300 [thread overview] Message-ID: <23ABB827-11CA-4A4E-AB5C-839BCA8F3A50@tarantool.org> (raw) In-Reply-To: <91edd7e8b72a93c5b5c0592c181576fca98e66fd.1561372731.git.roman.habibov@tarantool.org> > 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; > } > > +/* > + * Check that left node of @a expr with the collation in the root > + * can be used with <COLLATE>. 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 = expr->pLeft; > + while (left->op == TK_COLLATE) > + left = left->pLeft; > + enum field_type type; > + if (left->op == TK_COLUMN) { > + int col_num = left->iColumn; > + type = left->space_def->fields[col_num].type; > + } else { > + type = left->type; > + } > + if (type != FIELD_TYPE_STRING && type != FIELD_TYPE_SCALAR) { > + diag_set(ClientError, ER_SQL_PARSER_GENERIC, > + "COLLATE can't be used with " > + "non-string arguments”); -> COLLATE clause ... > + parse->is_aborted = 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 = sql_expr_type(pExpr->pLeft); > + if (check_collate_arg(pParse, pExpr) != 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 == TK_COLLATE && check_collate_arg(pParse, pExpr) != 0) > + return 0; Why this check is required (in addition to the same check in ExprCodeTarget)? > int r2; > pExpr = 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 = require("sqltester") > -test:plan(177) > +test:plan(191) > > local prefix = "collation-" > > @@ -546,4 +546,77 @@ test:do_execsql_test( > [[ SELECT s COLLATE "unicode_ci" FROM a ORDER BY s COLLATE "unicode" ]], > {"b","B"}) > > +-- 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’, BINARY is non-existent collation. Correct usage is “binary”. > +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( > -- </in3-1.13> > }) > > - > - > --- The first of these queries has to use the temp-table, because the > --- 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, { > - -- <in3-1.14> > - 1, 1, 3, 5 > - -- </in3-1.14> > - }) > - > -test:do_test( > - "in3-1.15", > - function() > - return exec_neph(" SELECT a FROM t1 WHERE a COLLATE \"binary\" IN (SELECT a FROM t1) ") > - end, { > - -- <in3-1.15> > - 1, 1, 3, 5 > - -- </in3-1.15> > - }) I’d better fix these tests rather than delete.
next prev parent reply other threads:[~2019-06-25 17:30 UTC|newest] Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <cover.1561372731.git.roman.habibov@tarantool.org> [not found] ` <49f3a1279811efc66c580c6039cf1b890ff37889.1561372731.git.roman.habibov@tarantool.org> 2019-06-25 17:30 ` [tarantool-patches] Re: [PATCH 1/2 v2] test: check that collations isn't ignored in SELECTs n.pettik 2019-06-25 17:55 ` Vladislav Shpilevoy 2019-06-28 13:11 ` Roman Khabibov 2019-06-28 13:13 ` Roman Khabibov 2019-07-02 17:21 ` n.pettik [not found] ` <91edd7e8b72a93c5b5c0592c181576fca98e66fd.1561372731.git.roman.habibov@tarantool.org> 2019-06-25 17:30 ` n.pettik [this message] 2019-06-28 12:57 ` [tarantool-patches] Re: [PATCH 2/2 v2] sql: allow <COLLATE> only for string-like args Roman Khabibov 2019-07-02 15:55 ` n.pettik 2019-07-06 1:04 ` Roman Khabibov 2019-07-08 13:38 ` n.pettik 2019-07-16 13:09 ` Kirill Yukhin
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=23ABB827-11CA-4A4E-AB5C-839BCA8F3A50@tarantool.org \ --to=korablev@tarantool.org \ --cc=roman.habibov@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='[tarantool-patches] Re: [PATCH 2/2 v2] sql: allow <COLLATE> only for string-like args' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox