* [tarantool-patches] Re: [PATCH 1/2 v2] test: check that collations isn't ignored in SELECTs [not found] ` <49f3a1279811efc66c580c6039cf1b890ff37889.1561372731.git.roman.habibov@tarantool.org> @ 2019-06-25 17:30 ` n.pettik 2019-06-25 17:55 ` Vladislav Shpilevoy 2019-06-28 13:11 ` Roman Khabibov 0 siblings, 2 replies; 11+ messages in thread From: n.pettik @ 2019-06-25 17:30 UTC (permalink / raw) To: tarantool-patches; +Cc: Roman Khabibov > On 24 Jun 2019, at 13:54, Roman Khabibov <roman.habibov@tarantool.org> wrote: > > Add test to check that a new collation isn't ignored regardless > of a name of a previous one in the following patterns of quries: Nit: quries -> queries > > SELECT s COLLATE "unicode_ci" FROM a ORDER BY s COLLATE “unicode_ci" Why do you consider this kind of query? What can be wrong with it? Specifying collation for result set members doesn’t make much sense btw. > > Also note: It is disallowed to compare strings with different > collations: ISO/IEC JTC 1/SC 32, Part 2: Foundation, page 531 > --- > src/box/sql/resolve.c | 7 +++++++ > test/sql-tap/collation.test.lua | 19 ++++++++++++++++++- > 2 files changed, 25 insertions(+), 1 deletion(-) > > diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c > index fdf3703da..348b3ea9a 100644 > --- a/src/box/sql/resolve.c > +++ b/src/box/sql/resolve.c > @@ -109,6 +109,13 @@ resolveAlias(Parse * pParse, /* Parsing context */ > return; > if (zType[0] != 'G') > incrAggFunctionDepth(pDup, nSubquery); > + /* > + * If there was typed more than one explicit collations in > + * query, it will be a sequence of left nodes with the > + * collations in a tree. There is nothing special about > + * keeping the sequence. Only one collation could be > + * stored, but the present solution is simpler. > + */ Do not understand how mentioned example is related to this code. I suppose you might mean example like: SELECT s COLLATE “unicode” COLLATE “binary” COLLATE “unicode_ci” … Is this syntax allowed by ANSI? If so, which one (first or last) collation must be used? > if (pExpr->op == TK_COLLATE) { > pDup = > sqlExprAddCollateString(pParse, pDup, pExpr->u.zToken); > diff --git a/test/sql-tap/collation.test.lua b/test/sql-tap/collation.test.lua > index 79547361c..f36540fc2 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(174) > +test:plan(177) > > local prefix = "collation-" > > @@ -529,4 +529,21 @@ test:do_catchsql_test( > 'CREATE TABLE test3 (a int, b int, c int, PRIMARY KEY (a, a COLLATE foo, b, c))', > {1, "Collation 'FOO' does not exist"}) > > +-- gh-3805 Check that collation is not ignored. Why these test cases are related to the mentioned ticket? Which collation might be ignored? I see two collations in two different parts of query. First one is used during sorting routines, second one is simply ignored (since values of resulting set are not supposed to be compared with anything else). > + > +test:do_execsql_test( > + "collation-2.6.0", > + [[ CREATE TABLE a (id INT PRIMARY KEY, s TEXT) ]], > + {}) > + > +test:do_execsql_test( > + "collation-2.6.1", > + [[ INSERT INTO a VALUES (1, 'B'), (2, 'b') ]], > + {}) > + > +test:do_execsql_test( > + "collation-2.6.2", > + [[ SELECT s COLLATE "unicode_ci" FROM a ORDER BY s COLLATE "unicode" ]], > + {"b","B”}) > + > test:finish_test() > -- > 2.20.1 (Apple Git-117) > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [tarantool-patches] Re: [PATCH 1/2 v2] test: check that collations isn't ignored in SELECTs 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 1 sibling, 0 replies; 11+ messages in thread From: Vladislav Shpilevoy @ 2019-06-25 17:55 UTC (permalink / raw) To: tarantool-patches, n.pettik; +Cc: Roman Khabibov On 25/06/2019 19:30, n.pettik wrote: > > >> On 24 Jun 2019, at 13:54, Roman Khabibov <roman.habibov@tarantool.org> wrote: >> >> Add test to check that a new collation isn't ignored regardless >> of a name of a previous one in the following patterns of quries: > > Nit: quries -> queries > >> >> SELECT s COLLATE "unicode_ci" FROM a ORDER BY s COLLATE “unicode_ci" > > Why do you consider this kind of query? What can be wrong with it? That example works, but when the collations are different, according to the standard the last one should be applied, as I remember when I last time checked. Before the patch a wrong collation was used when there are several of them. If under `this kind of query` you mean why 'order by'? - it is just the only request where we can reproduce that. I've tried to find a simpler test, but did not manage. > Specifying collation for result set members doesn’t make much sense btw. > >> >> Also note: It is disallowed to compare strings with different >> collations: ISO/IEC JTC 1/SC 32, Part 2: Foundation, page 531 >> --- >> src/box/sql/resolve.c | 7 +++++++ >> test/sql-tap/collation.test.lua | 19 ++++++++++++++++++- >> 2 files changed, 25 insertions(+), 1 deletion(-) >> >> diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c >> index fdf3703da..348b3ea9a 100644 >> --- a/src/box/sql/resolve.c >> +++ b/src/box/sql/resolve.c >> @@ -109,6 +109,13 @@ resolveAlias(Parse * pParse, /* Parsing context */ >> return; >> if (zType[0] != 'G') >> incrAggFunctionDepth(pDup, nSubquery); >> + /* >> + * If there was typed more than one explicit collations in >> + * query, it will be a sequence of left nodes with the >> + * collations in a tree. There is nothing special about >> + * keeping the sequence. Only one collation could be >> + * stored, but the present solution is simpler. >> + */ > > Do not understand how mentioned example is related to this code. > I suppose you might mean example like: > > SELECT s COLLATE “unicode” COLLATE “binary” COLLATE “unicode_ci” … > > Is this syntax allowed by ANSI? If so, which one (first or last) collation must be used? Yes, allowed, the last collation should be used. Probably, Roman, you should include in the commit message a reference to the standard (probably you did, I don't know - the first message in that thread is lost in my copy of our mailing list). > >> if (pExpr->op == TK_COLLATE) { >> pDup = >> sqlExprAddCollateString(pParse, pDup, pExpr->u.zToken); ^ permalink raw reply [flat|nested] 11+ messages in thread
* [tarantool-patches] Re: [PATCH 1/2 v2] test: check that collations isn't ignored in SELECTs 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 1 sibling, 2 replies; 11+ messages in thread From: Roman Khabibov @ 2019-06-28 13:11 UTC (permalink / raw) To: tarantool-patches; +Cc: n. pettik, Vladislav Shpilevoy Hi! Thanks for the review. > On Jun 25, 2019, at 8:30 PM, n.pettik <korablev@tarantool.org> wrote: > > > >> On 24 Jun 2019, at 13:54, Roman Khabibov <roman.habibov@tarantool.org> wrote: >> >> Add test to check that a new collation isn't ignored regardless >> of a name of a previous one in the following patterns of quries: > > Nit: quries -> queries Fixed. >> >> SELECT s COLLATE "unicode_ci" FROM a ORDER BY s COLLATE “unicode_ci" > > Why do you consider this kind of query? What can be wrong with it? > Specifying collation for result set members doesn’t make much sense btw. I consider it, because it didn’t work after my addition of checking from the second patch: +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 = sql_expr_type(left); + if (type != FIELD_TYPE_STRING && type != FIELD_TYPE_SCALAR) { + diag_set(ClientError, ER_SQL_PARSER_GENERIC, + "COLLATE clause can't be used with non-string " + "arguments"); + parse->is_aborted = true; + return -1; + } + return 0; +} The tree of this query included sequence of collations on left nodes. This is the result of optimizations in multiSelectOrderBy. Now I can not exactly say how It occurs. At first time, I didn’t use loop to descend throughout left nodes. I added this loop, because of that reason. >> Also note: It is disallowed to compare strings with different >> collations: ISO/IEC JTC 1/SC 32, Part 2: Foundation, page 531 >> --- >> src/box/sql/resolve.c | 7 +++++++ >> test/sql-tap/collation.test.lua | 19 ++++++++++++++++++- >> 2 files changed, 25 insertions(+), 1 deletion(-) >> >> diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c >> index fdf3703da..348b3ea9a 100644 >> --- a/src/box/sql/resolve.c >> +++ b/src/box/sql/resolve.c >> @@ -109,6 +109,13 @@ resolveAlias(Parse * pParse, /* Parsing context */ >> return; >> if (zType[0] != 'G') >> incrAggFunctionDepth(pDup, nSubquery); >> + /* >> + * If there was typed more than one explicit collations in >> + * query, it will be a sequence of left nodes with the >> + * collations in a tree. There is nothing special about >> + * keeping the sequence. Only one collation could be >> + * stored, but the present solution is simpler. >> + */ > > Do not understand how mentioned example is related to this code. > I suppose you might mean example like: > > SELECT s COLLATE “unicode” COLLATE “binary” COLLATE “unicode_ci” … > > Is this syntax allowed by ANSI? If so, which one (first or last) collation must be used? > >> if (pExpr->op == TK_COLLATE) { >> pDup = >> sqlExprAddCollateString(pParse, pDup, pExpr->u.zToken); >> diff --git a/test/sql-tap/collation.test.lua b/test/sql-tap/collation.test.lua >> index 79547361c..f36540fc2 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(174) >> +test:plan(177) >> >> local prefix = "collation-" >> >> @@ -529,4 +529,21 @@ test:do_catchsql_test( >> 'CREATE TABLE test3 (a int, b int, c int, PRIMARY KEY (a, a COLLATE foo, b, c))', >> {1, "Collation 'FOO' does not exist"}) >> >> +-- gh-3805 Check that collation is not ignored. > > Why these test cases are related to the mentioned ticket? > Which collation might be ignored? I see two collations in > two different parts of query. First one is used during sorting > routines, second one is simply ignored (since values of > resulting set are not supposed to be compared with anything else). See above. >> + >> +test:do_execsql_test( >> + "collation-2.6.0", >> + [[ CREATE TABLE a (id INT PRIMARY KEY, s TEXT) ]], >> + {}) >> + >> +test:do_execsql_test( >> + "collation-2.6.1", >> + [[ INSERT INTO a VALUES (1, 'B'), (2, 'b') ]], >> + {}) >> + >> +test:do_execsql_test( >> + "collation-2.6.2", >> + [[ SELECT s COLLATE "unicode_ci" FROM a ORDER BY s COLLATE "unicode" ]], >> + {"b","B”}) >> + >> test:finish_test() >> -- >> 2.20.1 (Apple Git-117) >> > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [tarantool-patches] Re: [PATCH 1/2 v2] test: check that collations isn't ignored in SELECTs 2019-06-28 13:11 ` Roman Khabibov @ 2019-06-28 13:13 ` Roman Khabibov 2019-07-02 17:21 ` n.pettik 1 sibling, 0 replies; 11+ messages in thread From: Roman Khabibov @ 2019-06-28 13:13 UTC (permalink / raw) To: tarantool-patches; +Cc: n. pettik Forgot. commit 40690ab014e80d954c0139506ed41b0aa18b95d5 Author: Roman Khabibov <roman.habibov@tarantool.org> Date: Mon May 6 14:45:51 2019 +0300 test: check that collations isn't ignored in SELECTs Add test to check that a new collation isn't ignored regardless of a name of a previous one in the following patterns of queries: SELECT s COLLATE "unicode_ci" FROM a ORDER BY s COLLATE "unicode_ci" Also note: It is disallowed to compare strings with different collations: ISO/IEC JTC 1/SC 32, Part 2: Foundation, page 531 diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c index fdf3703da..348b3ea9a 100644 --- a/src/box/sql/resolve.c +++ b/src/box/sql/resolve.c @@ -109,6 +109,13 @@ resolveAlias(Parse * pParse, /* Parsing context */ return; if (zType[0] != 'G') incrAggFunctionDepth(pDup, nSubquery); + /* + * If there was typed more than one explicit collations in + * query, it will be a sequence of left nodes with the + * collations in a tree. There is nothing special about + * keeping the sequence. Only one collation could be + * stored, but the present solution is simpler. + */ if (pExpr->op == TK_COLLATE) { pDup = sqlExprAddCollateString(pParse, pDup, pExpr->u.zToken); diff --git a/test/sql-tap/collation.test.lua b/test/sql-tap/collation.test.lua index 79547361c..f36540fc2 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(174) +test:plan(177) local prefix = "collation-" @@ -529,4 +529,21 @@ test:do_catchsql_test( 'CREATE TABLE test3 (a int, b int, c int, PRIMARY KEY (a, a COLLATE foo, b, c))', {1, "Collation 'FOO' does not exist"}) +-- gh-3805 Check that collation is not ignored. + +test:do_execsql_test( + "collation-2.6.0", + [[ CREATE TABLE a (id INT PRIMARY KEY, s TEXT) ]], + {}) + +test:do_execsql_test( + "collation-2.6.1", + [[ INSERT INTO a VALUES (1, 'B'), (2, 'b') ]], + {}) + +test:do_execsql_test( + "collation-2.6.2", + [[ SELECT s COLLATE "unicode_ci" FROM a ORDER BY s COLLATE "unicode" ]], + {"b","B"}) + test:finish_test() ^ permalink raw reply [flat|nested] 11+ messages in thread
* [tarantool-patches] Re: [PATCH 1/2 v2] test: check that collations isn't ignored in SELECTs 2019-06-28 13:11 ` Roman Khabibov 2019-06-28 13:13 ` Roman Khabibov @ 2019-07-02 17:21 ` n.pettik 1 sibling, 0 replies; 11+ messages in thread From: n.pettik @ 2019-07-02 17:21 UTC (permalink / raw) To: tarantool-patches; +Cc: Roman Khabibov [-- Attachment #1: Type: text/plain, Size: 4730 bytes --] > On 28 Jun 2019, at 16:11, Roman Khabibov <roman.habibov@tarantool.org> wrote: > > Hi! Thanks for the review. > >> On Jun 25, 2019, at 8:30 PM, n.pettik <korablev@tarantool.org <mailto:korablev@tarantool.org>> wrote: >>> On 24 Jun 2019, at 13:54, Roman Khabibov <roman.habibov@tarantool.org <mailto:roman.habibov@tarantool.org>> wrote: >>> >>> Add test to check that a new collation isn't ignored regardless >>> of a name of a previous one in the following patterns of quries: >> >> Nit: quries -> queries > Fixed. > >>> >>> SELECT s COLLATE "unicode_ci" FROM a ORDER BY s COLLATE “unicode_ci" >> >> Why do you consider this kind of query? What can be wrong with it? >> Specifying collation for result set members doesn’t make much sense btw. > I consider it, because it didn’t work after my addition of checking from the > second patch: > +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 = sql_expr_type(left); > + if (type != FIELD_TYPE_STRING && type != FIELD_TYPE_SCALAR) { > + diag_set(ClientError, ER_SQL_PARSER_GENERIC, > + "COLLATE clause can't be used with non-string " > + "arguments"); > + parse->is_aborted = true; > + return -1; > + } > + return 0; > +} > The tree of this query included sequence of collations on left nodes. This is > the result of optimizations in multiSelectOrderBy. Now I can not exactly say how > It occurs. At first time, I didn’t use loop to descend throughout left nodes. > I added this loop, because of that reason. Wait, in the example like this: SELECT s COLLATE "unicode_ci" FROM a ORDER BY s COLLATE “unicode_ci" collate clauses refer to independent parts of query and can’t contradict to each other. As I already said: "First one is used during sorting routines, second one is simply ignored (since values of resulting set are not supposed to be compared with anything else)." Why then are they organised into list? Please, investigate reasons of this “optimisation”. >>> Also note: It is disallowed to compare strings with different >>> collations: ISO/IEC JTC 1/SC 32, Part 2: Foundation, page 531 >>> --- >>> src/box/sql/resolve.c | 7 +++++++ >>> test/sql-tap/collation.test.lua | 19 ++++++++++++++++++- >>> 2 files changed, 25 insertions(+), 1 deletion(-) >>> >>> diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c >>> index fdf3703da..348b3ea9a 100644 >>> --- a/src/box/sql/resolve.c >>> +++ b/src/box/sql/resolve.c >>> @@ -109,6 +109,13 @@ resolveAlias(Parse * pParse, /* Parsing context */ >>> return; >>> if (zType[0] != 'G') >>> incrAggFunctionDepth(pDup, nSubquery); >>> + /* >>> + * If there was typed more than one explicit collations in >>> + * query, it will be a sequence of left nodes with the >>> + * collations in a tree. There is nothing special about >>> + * keeping the sequence. Only one collation could be >>> + * stored, but the present solution is simpler. >>> + */ >> >> Do not understand how mentioned example is related to this code. >> I suppose you might mean example like: >> >> SELECT s COLLATE “unicode” COLLATE “binary” COLLATE “unicode_ci” … >> >> Is this syntax allowed by ANSI? If so, which one (first or last) collation must be used? >> >>> if (pExpr->op == TK_COLLATE) { >>> pDup = >>> sqlExprAddCollateString(pParse, pDup, pExpr->u.zToken); >>> diff --git a/test/sql-tap/collation.test.lua b/test/sql-tap/collation.test.lua >>> index 79547361c..f36540fc2 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(174) >>> +test:plan(177) >>> >>> local prefix = "collation-" >>> >>> @@ -529,4 +529,21 @@ test:do_catchsql_test( >>> 'CREATE TABLE test3 (a int, b int, c int, PRIMARY KEY (a, a COLLATE foo, b, c))', >>> {1, "Collation 'FOO' does not exist"}) >>> >>> +-- gh-3805 Check that collation is not ignored. >> >> Why these test cases are related to the mentioned ticket? >> Which collation might be ignored? I see two collations in >> two different parts of query. First one is used during sorting >> routines, second one is simply ignored (since values of >> resulting set are not supposed to be compared with anything else). > See above. See above what?:) Please drop this commit or provide decent explanation. Now I can’t understand how provided test is related to the ticket and what’s wrong with this case at all. [-- Attachment #2: Type: text/html, Size: 39478 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <91edd7e8b72a93c5b5c0592c181576fca98e66fd.1561372731.git.roman.habibov@tarantool.org>]
* [tarantool-patches] Re: [PATCH 2/2 v2] sql: allow <COLLATE> only for string-like args [not found] ` <91edd7e8b72a93c5b5c0592c181576fca98e66fd.1561372731.git.roman.habibov@tarantool.org> @ 2019-06-25 17:30 ` n.pettik 2019-06-28 12:57 ` Roman Khabibov 2019-07-16 13:09 ` Kirill Yukhin 0 siblings, 2 replies; 11+ messages in thread From: n.pettik @ 2019-06-25 17:30 UTC (permalink / raw) To: tarantool-patches; +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; > } > > +/* > + * 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [tarantool-patches] Re: [PATCH 2/2 v2] sql: allow <COLLATE> only for string-like args 2019-06-25 17:30 ` [tarantool-patches] Re: [PATCH 2/2 v2] sql: allow <COLLATE> only for string-like args n.pettik @ 2019-06-28 12:57 ` Roman Khabibov 2019-07-02 15:55 ` n.pettik 2019-07-16 13:09 ` Kirill Yukhin 1 sibling, 1 reply; 11+ messages in thread From: Roman Khabibov @ 2019-06-28 12:57 UTC (permalink / raw) To: tarantool-patches; +Cc: n. pettik, Vladislav Shpilevoy Hi! Thanks for the review. > On Jun 25, 2019, at 8:30 PM, n.pettik <korablev@tarantool.org> wrote: > > >> 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 … +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 = sql_expr_type(left); + if (type != FIELD_TYPE_STRING && type != FIELD_TYPE_SCALAR) { + diag_set(ClientError, ER_SQL_PARSER_GENERIC, + "COLLATE clause can't be used with non-string " + "arguments"); + parse->is_aborted = true; + return -1; + } + return 0; +} + >> + 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)? It is needed e.g. for the following queries: “SELECT NOT TRUE COLLATE “unicode””. “NOT” will be on the root. The sequence of calls: sqlExprCodeTagret -> (case TK_NOT) -> sqlExprCodeTemp. NOT / \ unicode 0x0 / \ TRUE 0x0 >> 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”. @@ -546,4 +546,87 @@ 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', + {1, "COLLATE clause can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.7.2", + 'SELECT one COLLATE "unicode_ci" FROM test1', + {1, "COLLATE clause can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.7.3", + 'SELECT two COLLATE "binary" FROM test1', + {1, "COLLATE clause can't be used with non-string arguments"}) + + +test:do_catchsql_test( + "collation-2.7.4", + 'SELECT (one + two) COLLATE "binary" FROM test1', + {1, "COLLATE clause can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.7.5", + 'SELECT (SELECT one FROM test1) COLLATE "binary"', + {1, "COLLATE clause can't be used with non-string arguments"}) + +test:do_execsql_test( + "collation-2.7.6", + 'SELECT TRIM(\'A\') COLLATE "binary"', + {"A"}) + +test:do_catchsql_test( + "collation-2.7.7", + 'SELECT RANDOM() COLLATE "binary"', + {1, "COLLATE clause can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.7.8", + 'SELECT LENGTH(\'A\') COLLATE "binary"', + {1, "COLLATE clause can't be used with non-string arguments"}) + >> +test:do_catchsql_test( >> + "collation-2.7.14", >> + 'SELECT +\'str\' COLLATE \"unicode\"', >> + {0,{"str"}}) > > What about concatenation operation? + +test:do_execsql_test( + "collation-2.7.15", + 'SELECT (\'con\'||\'cat\') COLLATE "unicode"', + {"concat"}) + >> 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. Previous test in this file will be same if I remove COLLATE clauses. test:do_test( "in3-1.3", function() return exec_neph(" SELECT a FROM t1 WHERE a IN (SELECT a FROM t1); ") end, { -- <in3-1.3> 0, 1, 3, 5 -- </in3-1.3> }) commit b35caf9a05e5a4a01566756e8dda6b9621b9348b Author: Roman Khabibov <roman.habibov@tarantool.org> Date: Mon May 6 14:30:21 2019 +0300 sql: allow <COLLATE> only for string-like args Before this patch, user could use COLLATE with non-string-like literals, columns or subquery results. Disallow such usage. Closes #3804 diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index 898d16cf3..e1aae18fc 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -197,6 +197,34 @@ 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 = sql_expr_type(left); + if (type != FIELD_TYPE_STRING && type != FIELD_TYPE_SCALAR) { + diag_set(ClientError, ER_SQL_PARSER_GENERIC, + "COLLATE clause can't be used with non-string " + "arguments"); + 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 +4201,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 +4427,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; 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..99314ec04 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(193) local prefix = "collation-" @@ -546,4 +546,87 @@ 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', + {1, "COLLATE clause can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.7.2", + 'SELECT one COLLATE "unicode_ci" FROM test1', + {1, "COLLATE clause can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.7.3", + 'SELECT two COLLATE "binary" FROM test1', + {1, "COLLATE clause can't be used with non-string arguments"}) + + +test:do_catchsql_test( + "collation-2.7.4", + 'SELECT (one + two) COLLATE "binary" FROM test1', + {1, "COLLATE clause can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.7.5", + 'SELECT (SELECT one FROM test1) COLLATE "binary"', + {1, "COLLATE clause can't be used with non-string arguments"}) + +test:do_execsql_test( + "collation-2.7.6", + 'SELECT TRIM(\'A\') COLLATE "binary"', + {"A"}) + +test:do_catchsql_test( + "collation-2.7.7", + 'SELECT RANDOM() COLLATE "binary"', + {1, "COLLATE clause can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.7.8", + 'SELECT LENGTH(\'A\') COLLATE "binary"', + {1, "COLLATE clause can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.7.9", + 'SELECT TRUE COLLATE "unicode"', + {1, "COLLATE clause can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.7.10", + 'SELECT NOT TRUE COLLATE "unicode"', + {1, "COLLATE clause can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.7.11", + 'SELECT TRUE AND TRUE COLLATE "unicode"', + {1, "COLLATE clause can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.7.12", + 'SELECT 1 | 1 COLLATE "unicode"', + {1, "COLLATE clause can't be used with non-string arguments"}) + +test:do_execsql_test( + "collation-2.7.14", + 'SELECT +\'str\' COLLATE "unicode"', + {"str"}) + +test:do_execsql_test( + "collation-2.7.15", + 'SELECT (\'con\'||\'cat\') COLLATE "unicode"', + {"concat"}) + +test:do_execsql_test( + "collation-2.7.16", + 'SELECT (SELECT \'str\') COLLATE "binary" COLLATE "binary";', + {"str"}) + test:finish_test() diff --git a/test/sql-tap/distinct.test.lua b/test/sql-tap/distinct.test.lua index e6970084e..ae35a0db5 100755 --- a/test/sql-tap/distinct.test.lua +++ b/test/sql-tap/distinct.test.lua @@ -121,12 +121,12 @@ local data = { {"12.1", 0, "SELECT DISTINCT a, d FROM t1"}, {"12.2", 0, "SELECT DISTINCT a, d FROM t4"}, {"13.1", 0, "SELECT DISTINCT a, b, c COLLATE \"unicode_ci\" FROM t1"}, - {"13.2", 0, "SELECT DISTINCT a, b, c COLLATE \"unicode_ci\" FROM t4"}, + {"13.2", 1, "SELECT DISTINCT a, b, c FROM t4"}, {"14.1", 0, "SELECT DISTINCT a, d COLLATE \"unicode_ci\" FROM t1"}, {"14.2", 1, "SELECT DISTINCT a, d COLLATE \"unicode_ci\" FROM t4"}, {"15 ", 0, "SELECT DISTINCT a, d COLLATE \"binary\" FROM t1"}, {"16.1", 0, "SELECT DISTINCT a, b, c COLLATE \"binary\" FROM t1"}, - {"16.2", 0, "SELECT DISTINCT a, b, c COLLATE \"binary\" FROM t4"}, + {"16.2", 1, "SELECT DISTINCT a, b, c FROM t4"}, {"17", 0, --{ \/* Technically, it would be possible to detect that DISTINCT\n ** is a no-op in cases like the following. But sql does not\n ** do so. *\/\n "SELECT DISTINCT t1.id FROM t1, t2 WHERE t1.id=t2.x" }, {"18 ", 1, "SELECT DISTINCT c1, c2 FROM t3"}, @@ -173,7 +173,7 @@ data = { {"a, b, c FROM t1", {"btree"}, {"A", "B", "C", "a", "b", "c"}}, {"a, b, c FROM t1 ORDER BY a, b, c", {"btree"}, {"A", "B", "C", "a", "b", "c"}}, {"b FROM t1 WHERE a = 'a'", {}, {"b"}}, - {"b FROM t1 ORDER BY +b COLLATE \"binary\"", {"btree", "btree"}, {"B", "b"}}, + {"b FROM t1 ORDER BY b COLLATE \"binary\"", {"btree", "btree"}, {"B", "b"}}, {"a FROM t1", {}, {"A", "a"}}, {"b COLLATE \"unicode_ci\" FROM t1", {}, {"b"}}, {"b COLLATE \"unicode_ci\" FROM t1 ORDER BY b COLLATE \"unicode_ci\"", {}, {"b"}}, diff --git a/test/sql-tap/identifier_case.test.lua b/test/sql-tap/identifier_case.test.lua index 4db729f11..65ed9aea1 100755 --- a/test/sql-tap/identifier_case.test.lua +++ b/test/sql-tap/identifier_case.test.lua @@ -166,7 +166,7 @@ test:do_execsql_test( test:do_execsql_test( test_prefix.."4.1", - string.format([[select * from table1 order by a collate "unicode_ci"]]), + string.format([[select * from table1 order by a]]), {} ) 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 @@ -1,6 +1,6 @@ #!/usr/bin/env tarantool test = require("sqltester") -test:plan(27) +test:plan(25) --!./tcltestrunner.lua -- 2007 November 29 @@ -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> - }) - -- Neither of these queries require a temp-table. The collation sequence -- makes no difference when using a rowid. -- diff --git a/test/sql-tap/resolver01.test.lua b/test/sql-tap/resolver01.test.lua index 9fcf0c7c0..315c892ac 100755 --- a/test/sql-tap/resolver01.test.lua +++ b/test/sql-tap/resolver01.test.lua @@ -106,7 +106,7 @@ test:do_catchsql_test( test:do_catchsql_test( "resolver01-2.1", [[ - SELECT 2 AS y FROM t1, t2 ORDER BY y COLLATE "unicode_ci"; + SELECT 2 AS y FROM t1, t2 ORDER BY y; ]], { -- <resolver01-2.1> 0, {2} @@ -116,7 +116,7 @@ test:do_catchsql_test( test:do_catchsql_test( "resolver01-2.2", [[ - SELECT 2 AS yy FROM t1, t2 ORDER BY y COLLATE "unicode_ci"; + SELECT 2 AS yy FROM t1, t2 ORDER BY y; ]], { -- <resolver01-2.2> 1, "ambiguous column name: Y" @@ -126,7 +126,7 @@ test:do_catchsql_test( test:do_catchsql_test( "resolver01-2.3", [[ - SELECT x AS y FROM t3 ORDER BY y COLLATE "unicode_ci"; + SELECT x AS y FROM t3 ORDER BY y; ]], { -- <resolver01-2.3> 0, {11, 33} @@ -136,7 +136,7 @@ test:do_catchsql_test( test:do_catchsql_test( "resolver01-2.4", [[ - SELECT x AS yy FROM t3 ORDER BY y COLLATE "unicode_ci"; + SELECT x AS yy FROM t3 ORDER BY y; ]], { -- <resolver01-2.4> 0, {33, 11} @@ -146,7 +146,7 @@ test:do_catchsql_test( test:do_catchsql_test( "resolver01-2.5", [[ - SELECT x AS yy FROM t3 ORDER BY yy COLLATE "unicode_ci"; + SELECT x AS yy FROM t3 ORDER BY yy; ]], { -- <resolver01-2.5> 0, {11, 33} @@ -156,7 +156,7 @@ test:do_catchsql_test( test:do_catchsql_test( "resolver01-2.6", [[ - SELECT x AS yy FROM t3 ORDER BY 1 COLLATE "unicode_ci"; + SELECT x AS yy FROM t3 ORDER BY 1; ]], { -- <resolver01-2.6> 0, {11, 33} diff --git a/test/sql-tap/select1.test.lua b/test/sql-tap/select1.test.lua index 6beeb34cb..6811f7dcb 100755 --- a/test/sql-tap/select1.test.lua +++ b/test/sql-tap/select1.test.lua @@ -1672,7 +1672,7 @@ test:do_execsql_test( test:do_execsql_test( "select1-10.7", [[ - SELECT f1 COLLATE "unicode_ci" AS x FROM test1 ORDER BY x + SELECT f1 AS x FROM test1 ORDER BY x ]], { -- <select1-10.7> 11, 33 diff --git a/test/sql-tap/tkt-b75a9ca6b0.test.lua b/test/sql-tap/tkt-b75a9ca6b0.test.lua index 89817d2af..dde6ab502 100755 --- a/test/sql-tap/tkt-b75a9ca6b0.test.lua +++ b/test/sql-tap/tkt-b75a9ca6b0.test.lua @@ -1,6 +1,6 @@ #!/usr/bin/env tarantool test = require("sqltester") -test:plan(22) +test:plan(20) --!./tcltestrunner.lua -- 2014-04-21 @@ -57,7 +57,6 @@ local eqps = { {"SELECT x,y FROM t1 GROUP BY x, y ORDER BY x, y DESC", {1, 3, 2, 2, 3, 1}, {idxscan, sort}}, {"SELECT x,y FROM t1 GROUP BY x, y ORDER BY x DESC, y DESC", {3, 1, 2, 2, 1, 3}, {idxscan, sort}}, {"SELECT x,y FROM t1 GROUP BY x, y ORDER BY x ASC, y ASC", {1, 3, 2, 2, 3, 1}, {idxscan}}, - {"SELECT x,y FROM t1 GROUP BY x, y ORDER BY x COLLATE \"unicode_ci\", y", {1, 3, 2, 2, 3, 1}, {idxscan, sort}}, } for tn, val in ipairs(eqps) do local q = val[1] diff --git a/test/sql-tap/with1.test.lua b/test/sql-tap/with1.test.lua index ec45e5e76..6985c589e 100755 --- a/test/sql-tap/with1.test.lua +++ b/test/sql-tap/with1.test.lua @@ -1043,7 +1043,7 @@ test:do_catchsql_test(13.3, [[ -- 2015-04-12 -- test:do_execsql_test(14.1, [[ - WITH x AS (SELECT * FROM t) SELECT 0 EXCEPT SELECT 0 ORDER BY 1 COLLATE "binary"; + WITH x AS (SELECT * FROM t) SELECT 0 EXCEPT SELECT 0 ORDER BY 1; ]], { -- <14.1> ^ permalink raw reply [flat|nested] 11+ messages in thread
* [tarantool-patches] Re: [PATCH 2/2 v2] sql: allow <COLLATE> only for string-like args 2019-06-28 12:57 ` Roman Khabibov @ 2019-07-02 15:55 ` n.pettik 2019-07-06 1:04 ` Roman Khabibov 0 siblings, 1 reply; 11+ messages in thread From: n.pettik @ 2019-07-02 15:55 UTC (permalink / raw) To: tarantool-patches; +Cc: Roman Khabibov >>> @@ -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)? > It is needed e.g. for the following queries: “SELECT NOT TRUE COLLATE “unicode””. > “NOT” will be on the root. The sequence of calls: sqlExprCodeTagret -> (case TK_NOT) > -> sqlExprCodeTemp. > NOT > / \ > unicode 0x0 > / \ > TRUE 0x0 >>> int r2; >>> pExpr = 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 == TK_COLLATE && check_collate_arg(pParse, pExpr) != 0) - return 0; int r2; - pExpr = sqlExprSkipCollate(pExpr); if (ConstFactorOk(pParse) && pExpr->op != TK_REGISTER && sqlExprIsConstantNotJoin(pExpr) ) { What is more, you don’t 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 = sql_expr_type(pExpr->pLeft); if (check_collate_arg(pParse, pExpr) != 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( >>> -- </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. > > Previous test in this file will be same if I remove COLLATE clauses. You don’t 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [tarantool-patches] Re: [PATCH 2/2 v2] sql: allow <COLLATE> only for string-like args 2019-07-02 15:55 ` n.pettik @ 2019-07-06 1:04 ` Roman Khabibov 2019-07-08 13:38 ` n.pettik 0 siblings, 1 reply; 11+ messages in thread From: Roman Khabibov @ 2019-07-06 1:04 UTC (permalink / raw) To: tarantool-patches; +Cc: n. pettik Hi! Thanks for the review. I decided to drop first commit. > On Jul 2, 2019, at 6:55 PM, n.pettik <korablev@tarantool.org> wrote: > > >>>> @@ -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)? >> It is needed e.g. for the following queries: “SELECT NOT TRUE COLLATE “unicode””. >> “NOT” will be on the root. The sequence of calls: sqlExprCodeTagret -> (case TK_NOT) >> -> sqlExprCodeTemp. >> NOT >> / \ >> unicode 0x0 >> / \ >> TRUE 0x0 >>>> int r2; >>>> pExpr = 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 == TK_COLLATE && check_collate_arg(pParse, pExpr) != 0) > - return 0; > int r2; > - pExpr = sqlExprSkipCollate(pExpr); > if (ConstFactorOk(pParse) > && pExpr->op != TK_REGISTER && sqlExprIsConstantNotJoin(pExpr) > ) { @@ -4397,7 +4427,6 @@ int sqlExprCodeTemp(Parse * pParse, Expr * pExpr, int *pReg) { int r2; - pExpr = sqlExprSkipCollate(pExpr); if (ConstFactorOk(pParse) && pExpr->op != TK_REGISTER && sqlExprIsConstantNotJoin(pExpr) ) { > What is more, you don’t 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 = sql_expr_type(pExpr->pLeft); > if (check_collate_arg(pParse, pExpr) != 0) > break; > return sqlExprCodeTarget(pParse, pExpr->pLeft, @@ -4173,6 +4201,8 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) } case TK_SPAN: case TK_COLLATE:{ + if (check_collate_arg(pParse, pExpr) != 0) + break; return sqlExprCodeTarget(pParse, pExpr->pLeft, target); } >>>> 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. >> >> Previous test in this file will be same if I remove COLLATE clauses. > > You don’t 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. diff --git a/test/sql-tap/in3.test.lua b/test/sql-tap/in3.test.lua index 1ca6a6446..67883d2a0 100755 --- a/test/sql-tap/in3.test.lua +++ b/test/sql-tap/in3.test.lua @@ -1,6 +1,6 @@ #!/usr/bin/env tarantool test = require("sqltester") -test:plan(27) +test:plan(29) --!./tcltestrunner.lua -- 2007 November 29 @@ -186,7 +186,17 @@ test:do_test( -- </in3-1.13> }) - +test:do_execsql_test( + "in3-1.14", + [[ + CREATE TABLE t2(a TEXT PRIMARY KEY); + INSERT INTO t2 VALUES('A'); + INSERT INTO t2 VALUES('B'); + INSERT INTO t2 VALUES('C'); + ]], { + -- <in3-1.14> + -- </in3-1.14> + }) -- 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 @@ -194,23 +204,32 @@ test:do_test( -- require a temp-table, because the collation sequences match. -- test:do_test( - "in3-1.14", + "in3-1.15", function() - return exec_neph(" SELECT a FROM t1 WHERE a COLLATE \"unicode_ci\" IN (SELECT a FROM t1) ") + return exec_neph(" SELECT a FROM t2 WHERE a COLLATE \"unicode_ci\" IN (SELECT a FROM t2) ") end, { - -- <in3-1.14> - 1, 1, 3, 5 - -- </in3-1.14> + -- <in3-1.15> + 1, "A", "B", "C" + -- </in3-1.15> }) test:do_test( - "in3-1.15", + "in3-1.16", function() - return exec_neph(" SELECT a FROM t1 WHERE a COLLATE \"binary\" IN (SELECT a FROM t1) ") + return exec_neph(" SELECT a FROM t2 WHERE a COLLATE \"binary\" IN (SELECT a FROM t2) ") end, { - -- <in3-1.15> - 1, 1, 3, 5 - -- </in3-1.15> + -- <in3-1.16> + 1, "A", "B", "C" + -- </in3-1.16> + }) + +test:do_execsql_test( + "in3-1.17", + [[ + DROP TABLE t2 + ]], { + -- <in3-1.17> + -- </in3-1.17> }) commit 7a000757faeab3e2fddefb02b3dc65ef6398c17f Author: Roman Khabibov <roman.habibov@tarantool.org> Date: Mon May 6 14:30:21 2019 +0300 sql: allow <COLLATE> only for string-like args Before this patch, user could use COLLATE with non-string-like literals, columns or subquery results. Disallow such usage. Closes #3804 diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index 898d16cf3..96824e8e3 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -197,6 +197,34 @@ 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 = sql_expr_type(left); + if (type != FIELD_TYPE_STRING && type != FIELD_TYPE_SCALAR) { + diag_set(ClientError, ER_SQL_PARSER_GENERIC, + "COLLATE clause can't be used with non-string " + "arguments"); + 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 +4201,8 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) } case TK_SPAN: case TK_COLLATE:{ + if (check_collate_arg(pParse, pExpr) != 0) + break; return sqlExprCodeTarget(pParse, pExpr->pLeft, target); } @@ -4397,7 +4427,6 @@ int sqlExprCodeTemp(Parse * pParse, Expr * pExpr, int *pReg) { int r2; - pExpr = sqlExprSkipCollate(pExpr); if (ConstFactorOk(pParse) && pExpr->op != TK_REGISTER && sqlExprIsConstantNotJoin(pExpr) ) { diff --git a/test/sql-tap/collation.test.lua b/test/sql-tap/collation.test.lua index 79547361c..edf4a28b9 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(174) +test:plan(190) local prefix = "collation-" @@ -529,4 +529,87 @@ test:do_catchsql_test( 'CREATE TABLE test3 (a int, b int, c int, PRIMARY KEY (a, a COLLATE foo, b, c))', {1, "Collation 'FOO' does not exist"}) +-- 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', + {1, "COLLATE clause can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.7.2", + 'SELECT one COLLATE "unicode_ci" FROM test1', + {1, "COLLATE clause can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.7.3", + 'SELECT two COLLATE "binary" FROM test1', + {1, "COLLATE clause can't be used with non-string arguments"}) + + +test:do_catchsql_test( + "collation-2.7.4", + 'SELECT (one + two) COLLATE "binary" FROM test1', + {1, "COLLATE clause can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.7.5", + 'SELECT (SELECT one FROM test1) COLLATE "binary"', + {1, "COLLATE clause can't be used with non-string arguments"}) + +test:do_execsql_test( + "collation-2.7.6", + 'SELECT TRIM(\'A\') COLLATE "binary"', + {"A"}) + +test:do_catchsql_test( + "collation-2.7.7", + 'SELECT RANDOM() COLLATE "binary"', + {1, "COLLATE clause can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.7.8", + 'SELECT LENGTH(\'A\') COLLATE "binary"', + {1, "COLLATE clause can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.7.9", + 'SELECT TRUE COLLATE "unicode"', + {1, "COLLATE clause can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.7.10", + 'SELECT NOT TRUE COLLATE "unicode"', + {1, "COLLATE clause can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.7.11", + 'SELECT TRUE AND TRUE COLLATE "unicode"', + {1, "COLLATE clause can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.7.12", + 'SELECT 1 | 1 COLLATE "unicode"', + {1, "COLLATE clause can't be used with non-string arguments"}) + +test:do_execsql_test( + "collation-2.7.14", + 'SELECT +\'str\' COLLATE "unicode"', + {"str"}) + +test:do_execsql_test( + "collation-2.7.15", + 'SELECT (\'con\'||\'cat\') COLLATE "unicode"', + {"concat"}) + +test:do_execsql_test( + "collation-2.7.16", + 'SELECT (SELECT \'str\') COLLATE "binary" COLLATE "binary";', + {"str"}) + test:finish_test() diff --git a/test/sql-tap/distinct.test.lua b/test/sql-tap/distinct.test.lua index e6970084e..ae35a0db5 100755 --- a/test/sql-tap/distinct.test.lua +++ b/test/sql-tap/distinct.test.lua @@ -121,12 +121,12 @@ local data = { {"12.1", 0, "SELECT DISTINCT a, d FROM t1"}, {"12.2", 0, "SELECT DISTINCT a, d FROM t4"}, {"13.1", 0, "SELECT DISTINCT a, b, c COLLATE \"unicode_ci\" FROM t1"}, - {"13.2", 0, "SELECT DISTINCT a, b, c COLLATE \"unicode_ci\" FROM t4"}, + {"13.2", 1, "SELECT DISTINCT a, b, c FROM t4"}, {"14.1", 0, "SELECT DISTINCT a, d COLLATE \"unicode_ci\" FROM t1"}, {"14.2", 1, "SELECT DISTINCT a, d COLLATE \"unicode_ci\" FROM t4"}, {"15 ", 0, "SELECT DISTINCT a, d COLLATE \"binary\" FROM t1"}, {"16.1", 0, "SELECT DISTINCT a, b, c COLLATE \"binary\" FROM t1"}, - {"16.2", 0, "SELECT DISTINCT a, b, c COLLATE \"binary\" FROM t4"}, + {"16.2", 1, "SELECT DISTINCT a, b, c FROM t4"}, {"17", 0, --{ \/* Technically, it would be possible to detect that DISTINCT\n ** is a no-op in cases like the following. But sql does not\n ** do so. *\/\n "SELECT DISTINCT t1.id FROM t1, t2 WHERE t1.id=t2.x" }, {"18 ", 1, "SELECT DISTINCT c1, c2 FROM t3"}, @@ -173,7 +173,7 @@ data = { {"a, b, c FROM t1", {"btree"}, {"A", "B", "C", "a", "b", "c"}}, {"a, b, c FROM t1 ORDER BY a, b, c", {"btree"}, {"A", "B", "C", "a", "b", "c"}}, {"b FROM t1 WHERE a = 'a'", {}, {"b"}}, - {"b FROM t1 ORDER BY +b COLLATE \"binary\"", {"btree", "btree"}, {"B", "b"}}, + {"b FROM t1 ORDER BY b COLLATE \"binary\"", {"btree", "btree"}, {"B", "b"}}, {"a FROM t1", {}, {"A", "a"}}, {"b COLLATE \"unicode_ci\" FROM t1", {}, {"b"}}, {"b COLLATE \"unicode_ci\" FROM t1 ORDER BY b COLLATE \"unicode_ci\"", {}, {"b"}}, diff --git a/test/sql-tap/identifier_case.test.lua b/test/sql-tap/identifier_case.test.lua index 4db729f11..65ed9aea1 100755 --- a/test/sql-tap/identifier_case.test.lua +++ b/test/sql-tap/identifier_case.test.lua @@ -166,7 +166,7 @@ test:do_execsql_test( test:do_execsql_test( test_prefix.."4.1", - string.format([[select * from table1 order by a collate "unicode_ci"]]), + string.format([[select * from table1 order by a]]), {} ) diff --git a/test/sql-tap/in3.test.lua b/test/sql-tap/in3.test.lua index 1ca6a6446..67883d2a0 100755 --- a/test/sql-tap/in3.test.lua +++ b/test/sql-tap/in3.test.lua @@ -1,6 +1,6 @@ #!/usr/bin/env tarantool test = require("sqltester") -test:plan(27) +test:plan(29) --!./tcltestrunner.lua -- 2007 November 29 @@ -186,7 +186,17 @@ test:do_test( -- </in3-1.13> }) - +test:do_execsql_test( + "in3-1.14", + [[ + CREATE TABLE t2(a TEXT PRIMARY KEY); + INSERT INTO t2 VALUES('A'); + INSERT INTO t2 VALUES('B'); + INSERT INTO t2 VALUES('C'); + ]], { + -- <in3-1.14> + -- </in3-1.14> + }) -- 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 @@ -194,23 +204,32 @@ test:do_test( -- require a temp-table, because the collation sequences match. -- test:do_test( - "in3-1.14", + "in3-1.15", function() - return exec_neph(" SELECT a FROM t1 WHERE a COLLATE \"unicode_ci\" IN (SELECT a FROM t1) ") + return exec_neph(" SELECT a FROM t2 WHERE a COLLATE \"unicode_ci\" IN (SELECT a FROM t2) ") end, { - -- <in3-1.14> - 1, 1, 3, 5 - -- </in3-1.14> + -- <in3-1.15> + 1, "A", "B", "C" + -- </in3-1.15> }) test:do_test( - "in3-1.15", + "in3-1.16", function() - return exec_neph(" SELECT a FROM t1 WHERE a COLLATE \"binary\" IN (SELECT a FROM t1) ") + return exec_neph(" SELECT a FROM t2 WHERE a COLLATE \"binary\" IN (SELECT a FROM t2) ") end, { - -- <in3-1.15> - 1, 1, 3, 5 - -- </in3-1.15> + -- <in3-1.16> + 1, "A", "B", "C" + -- </in3-1.16> + }) + +test:do_execsql_test( + "in3-1.17", + [[ + DROP TABLE t2 + ]], { + -- <in3-1.17> + -- </in3-1.17> }) -- Neither of these queries require a temp-table. The collation sequence diff --git a/test/sql-tap/resolver01.test.lua b/test/sql-tap/resolver01.test.lua index 9fcf0c7c0..315c892ac 100755 --- a/test/sql-tap/resolver01.test.lua +++ b/test/sql-tap/resolver01.test.lua @@ -106,7 +106,7 @@ test:do_catchsql_test( test:do_catchsql_test( "resolver01-2.1", [[ - SELECT 2 AS y FROM t1, t2 ORDER BY y COLLATE "unicode_ci"; + SELECT 2 AS y FROM t1, t2 ORDER BY y; ]], { -- <resolver01-2.1> 0, {2} @@ -116,7 +116,7 @@ test:do_catchsql_test( test:do_catchsql_test( "resolver01-2.2", [[ - SELECT 2 AS yy FROM t1, t2 ORDER BY y COLLATE "unicode_ci"; + SELECT 2 AS yy FROM t1, t2 ORDER BY y; ]], { -- <resolver01-2.2> 1, "ambiguous column name: Y" @@ -126,7 +126,7 @@ test:do_catchsql_test( test:do_catchsql_test( "resolver01-2.3", [[ - SELECT x AS y FROM t3 ORDER BY y COLLATE "unicode_ci"; + SELECT x AS y FROM t3 ORDER BY y; ]], { -- <resolver01-2.3> 0, {11, 33} @@ -136,7 +136,7 @@ test:do_catchsql_test( test:do_catchsql_test( "resolver01-2.4", [[ - SELECT x AS yy FROM t3 ORDER BY y COLLATE "unicode_ci"; + SELECT x AS yy FROM t3 ORDER BY y; ]], { -- <resolver01-2.4> 0, {33, 11} @@ -146,7 +146,7 @@ test:do_catchsql_test( test:do_catchsql_test( "resolver01-2.5", [[ - SELECT x AS yy FROM t3 ORDER BY yy COLLATE "unicode_ci"; + SELECT x AS yy FROM t3 ORDER BY yy; ]], { -- <resolver01-2.5> 0, {11, 33} @@ -156,7 +156,7 @@ test:do_catchsql_test( test:do_catchsql_test( "resolver01-2.6", [[ - SELECT x AS yy FROM t3 ORDER BY 1 COLLATE "unicode_ci"; + SELECT x AS yy FROM t3 ORDER BY 1; ]], { -- <resolver01-2.6> 0, {11, 33} diff --git a/test/sql-tap/select1.test.lua b/test/sql-tap/select1.test.lua index 6beeb34cb..6811f7dcb 100755 --- a/test/sql-tap/select1.test.lua +++ b/test/sql-tap/select1.test.lua @@ -1672,7 +1672,7 @@ test:do_execsql_test( test:do_execsql_test( "select1-10.7", [[ - SELECT f1 COLLATE "unicode_ci" AS x FROM test1 ORDER BY x + SELECT f1 AS x FROM test1 ORDER BY x ]], { -- <select1-10.7> 11, 33 diff --git a/test/sql-tap/tkt-b75a9ca6b0.test.lua b/test/sql-tap/tkt-b75a9ca6b0.test.lua index 89817d2af..dde6ab502 100755 --- a/test/sql-tap/tkt-b75a9ca6b0.test.lua +++ b/test/sql-tap/tkt-b75a9ca6b0.test.lua @@ -1,6 +1,6 @@ #!/usr/bin/env tarantool test = require("sqltester") -test:plan(22) +test:plan(20) --!./tcltestrunner.lua -- 2014-04-21 @@ -57,7 +57,6 @@ local eqps = { {"SELECT x,y FROM t1 GROUP BY x, y ORDER BY x, y DESC", {1, 3, 2, 2, 3, 1}, {idxscan, sort}}, {"SELECT x,y FROM t1 GROUP BY x, y ORDER BY x DESC, y DESC", {3, 1, 2, 2, 1, 3}, {idxscan, sort}}, {"SELECT x,y FROM t1 GROUP BY x, y ORDER BY x ASC, y ASC", {1, 3, 2, 2, 3, 1}, {idxscan}}, - {"SELECT x,y FROM t1 GROUP BY x, y ORDER BY x COLLATE \"unicode_ci\", y", {1, 3, 2, 2, 3, 1}, {idxscan, sort}}, } for tn, val in ipairs(eqps) do local q = val[1] diff --git a/test/sql-tap/with1.test.lua b/test/sql-tap/with1.test.lua index ec45e5e76..6985c589e 100755 --- a/test/sql-tap/with1.test.lua +++ b/test/sql-tap/with1.test.lua @@ -1043,7 +1043,7 @@ test:do_catchsql_test(13.3, [[ -- 2015-04-12 -- test:do_execsql_test(14.1, [[ - WITH x AS (SELECT * FROM t) SELECT 0 EXCEPT SELECT 0 ORDER BY 1 COLLATE "binary"; + WITH x AS (SELECT * FROM t) SELECT 0 EXCEPT SELECT 0 ORDER BY 1; ]], { -- <14.1> ^ permalink raw reply [flat|nested] 11+ messages in thread
* [tarantool-patches] Re: [PATCH 2/2 v2] sql: allow <COLLATE> only for string-like args 2019-07-06 1:04 ` Roman Khabibov @ 2019-07-08 13:38 ` n.pettik 0 siblings, 0 replies; 11+ messages in thread From: n.pettik @ 2019-07-08 13:38 UTC (permalink / raw) To: tarantool-patches; +Cc: Roman Khabibov Lgtm. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [tarantool-patches] Re: [PATCH 2/2 v2] sql: allow <COLLATE> only for string-like args 2019-06-25 17:30 ` [tarantool-patches] Re: [PATCH 2/2 v2] sql: allow <COLLATE> only for string-like args n.pettik 2019-06-28 12:57 ` Roman Khabibov @ 2019-07-16 13:09 ` Kirill Yukhin 1 sibling, 0 replies; 11+ messages in thread From: Kirill Yukhin @ 2019-07-16 13:09 UTC (permalink / raw) To: tarantool-patches; +Cc: Roman Khabibov Hello, I've checked your patch into master. -- Regards, Kirill Yukhin ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-07-16 13:09 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [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 ` [tarantool-patches] Re: [PATCH 2/2 v2] sql: allow <COLLATE> only for string-like args n.pettik 2019-06-28 12:57 ` 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
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox