* [tarantool-patches] [PATCH 0/2] sql: allow <COLLATE> only for string-like args @ 2019-05-08 11:29 Roman Khabibov 2019-05-08 11:29 ` [tarantool-patches] [PATCH 1/2] sql: fix collation node duplication in AST Roman Khabibov 2019-05-08 11:29 ` [tarantool-patches] [PATCH 2/2] sql: allow <COLLATE> only for string-like args Roman Khabibov 0 siblings, 2 replies; 14+ messages in thread From: Roman Khabibov @ 2019-05-08 11:29 UTC (permalink / raw) To: tarantool-patches; +Cc: v.shpilevoy This patchset disallows use of <COLLATE> clause with non-string-like argumennts, that can be litherals, table columns, expressions with different operators or compound subquiries. During the work on this issue I found strange behaviour: node with explicit collation in the certain kind of query (more in the patch) was duplicated as left node. I think, that it is a bug, so I have fixed that. Roman Khabibov (2): sql: fix collation node duplication in AST sql: allow <COLLATE> only for string-like args src/box/sql/expr.c | 16 +++++++++ src/box/sql/resolve.c | 2 +- test/sql-tap/collation.test.lua | 50 ++++++++++++++++++++++++++- test/sql-tap/distinct.test.lua | 8 ++--- test/sql-tap/e_select1.test.lua | 2 +- test/sql-tap/identifier_case.test.lua | 2 +- test/sql-tap/resolver01.test.lua | 12 +++---- test/sql-tap/select1.test.lua | 2 +- test/sql-tap/selectE.test.lua | 15 +++----- test/sql-tap/with1.test.lua | 2 +- 10 files changed, 85 insertions(+), 26 deletions(-) -- Issue: https://github.com/tarantool/tarantool/issues/3804 Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/gh-3804-collate 2.20.1 (Apple Git-117) ^ permalink raw reply [flat|nested] 14+ messages in thread
* [tarantool-patches] [PATCH 1/2] sql: fix collation node duplication in AST 2019-05-08 11:29 [tarantool-patches] [PATCH 0/2] sql: allow <COLLATE> only for string-like args Roman Khabibov @ 2019-05-08 11:29 ` Roman Khabibov 2019-05-12 16:32 ` [tarantool-patches] " Vladislav Shpilevoy 2019-05-08 11:29 ` [tarantool-patches] [PATCH 2/2] sql: allow <COLLATE> only for string-like args Roman Khabibov 1 sibling, 1 reply; 14+ messages in thread From: Roman Khabibov @ 2019-05-08 11:29 UTC (permalink / raw) To: tarantool-patches; +Cc: v.shpilevoy Before this patch, if SELECT was used with compound query, COLLATION clauses and ORDER BY clause, C function resolveAlias built extra node with collation name into OrderBy's AST. E.g., for query: SELECT a COLLATE "unicode_ci" FROM t2 EXCEPT SELECT a FROM t3 ORDER BY 1 was appeared something like: "unicode_ci" / \ extra node -> "unicode_ci" NULL / \ "A" NULL / \ NULL NULL Needed for #3804 --- src/box/sql/resolve.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c index 504096e6d..c2c597dc0 100644 --- a/src/box/sql/resolve.c +++ b/src/box/sql/resolve.c @@ -109,7 +109,7 @@ resolveAlias(Parse * pParse, /* Parsing context */ return; if (zType[0] != 'G') incrAggFunctionDepth(pDup, nSubquery); - if (pExpr->op == TK_COLLATE) { + if (pExpr->op == TK_COLLATE && pDup->op != TK_COLLATE) { pDup = sqlExprAddCollateString(pParse, pDup, pExpr->u.zToken); } -- 2.20.1 (Apple Git-117) ^ permalink raw reply [flat|nested] 14+ messages in thread
* [tarantool-patches] Re: [PATCH 1/2] sql: fix collation node duplication in AST 2019-05-08 11:29 ` [tarantool-patches] [PATCH 1/2] sql: fix collation node duplication in AST Roman Khabibov @ 2019-05-12 16:32 ` Vladislav Shpilevoy 2019-05-28 14:10 ` Roman Khabibov 0 siblings, 1 reply; 14+ messages in thread From: Vladislav Shpilevoy @ 2019-05-12 16:32 UTC (permalink / raw) To: Roman Khabibov, tarantool-patches Hi! Thanks for the patch! Looks like the problem in the patch is much easier to reproduce, without 'EXCEPT': box.execute('SELECT s COLLATE "unicode_ci" FROM a ORDER BY 1 COLLATE "unicode_ci"') Just use ORDER BY <number> + COLLATE. But having got this simpler test I found that you broke another one. Look at this schema: box.cfg{} box.execute('CREATE TABLE a (id INT PRIMARY KEY, s TEXT)') box.execute("INSERT INTO a VALUES (1, 'B'), (2, 'b')") Before your patch: tarantool> box.execute('SELECT s COLLATE "unicode_ci" FROM a ORDER BY 1 COLLATE "unicode"') --- - metadata: - name: s COLLATE "unicode_ci" type: string rows: - ['b'] - ['B'] ... After your patch: tarantool> box.execute('SELECT s COLLATE "unicode_ci" FROM a ORDER BY 1 COLLATE "unicode"') --- - metadata: - name: s COLLATE "unicode_ci" type: string rows: - ['B'] - ['b'] ... Result set order has changed. This is because you ignore a new collation regardless of a name of a previous one. It means, that the patch should not be applied as is. We should either 1) Replace the old collation with a new one, as an optimization; 2) Left as is now. I vote for the second as the simplest, and add a test provided by me above to ensure we will never break it accidentally. This commit should consist of the test and a comment in resolveAlias. And keep this nice ASCII schema. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [tarantool-patches] Re: [PATCH 1/2] sql: fix collation node duplication in AST 2019-05-12 16:32 ` [tarantool-patches] " Vladislav Shpilevoy @ 2019-05-28 14:10 ` Roman Khabibov 2019-06-02 17:09 ` Vladislav Shpilevoy 0 siblings, 1 reply; 14+ messages in thread From: Roman Khabibov @ 2019-05-28 14:10 UTC (permalink / raw) To: tarantool-patches; +Cc: Vladislav Shpilevoy Hi! Thanks for the review. > I vote for the second as the simplest, and add a test provided by me > above to ensure we will never break it accidentally. > > This commit should consist of the test and a comment in resolveAlias. > And keep this nice ASCII schema. diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c index 504096e6d..f952d8c04 100644 --- a/src/box/sql/resolve.c +++ b/src/box/sql/resolve.c @@ -109,6 +109,11 @@ 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. + */ 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 0bf54576d..3c5d3053a 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. Must pass. + +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] 14+ messages in thread
* [tarantool-patches] Re: [PATCH 1/2] sql: fix collation node duplication in AST 2019-05-28 14:10 ` Roman Khabibov @ 2019-06-02 17:09 ` Vladislav Shpilevoy 2019-06-04 14:00 ` Roman Khabibov 0 siblings, 1 reply; 14+ messages in thread From: Vladislav Shpilevoy @ 2019-06-02 17:09 UTC (permalink / raw) To: Roman Khabibov, tarantool-patches Hi! Thanks for the fixes! See 2 comments below. On 28/05/2019 17:10, Roman Khabibov wrote: > Hi! Thanks for the review. > >> I vote for the second as the simplest, and add a test provided by me >> above to ensure we will never break it accidentally. >> >> This commit should consist of the test and a comment in resolveAlias. >> And keep this nice ASCII schema. > > diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c > index 504096e6d..f952d8c04 100644 > --- a/src/box/sql/resolve.c > +++ b/src/box/sql/resolve.c > @@ -109,6 +109,11 @@ 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. > + */ 1. Please, add a comment, that 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 0bf54576d..3c5d3053a 100755 > --- a/test/sql-tap/collation.test.lua > +++ b/test/sql-tap/collation.test.lua > @@ -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. Must pass. 2. Of course it must pass. It is the purpose of test. Please, drop 'Must pass'. > + > +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] 14+ messages in thread
* [tarantool-patches] Re: [PATCH 1/2] sql: fix collation node duplication in AST 2019-06-02 17:09 ` Vladislav Shpilevoy @ 2019-06-04 14:00 ` Roman Khabibov 0 siblings, 0 replies; 14+ messages in thread From: Roman Khabibov @ 2019-06-04 14:00 UTC (permalink / raw) To: tarantool-patches; +Cc: Vladislav Shpilevoy Hello, thanks for the review. > On Jun 2, 2019, at 8:09 PM, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote: > > Hi! Thanks for the fixes! See 2 comments below. > > On 28/05/2019 17:10, Roman Khabibov wrote: >> Hi! Thanks for the review. >> >>> I vote for the second as the simplest, and add a test provided by me >>> above to ensure we will never break it accidentally. >>> >>> This commit should consist of the test and a comment in resolveAlias. >>> And keep this nice ASCII schema. >> >> diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c >> index 504096e6d..f952d8c04 100644 >> --- a/src/box/sql/resolve.c >> +++ b/src/box/sql/resolve.c >> @@ -109,6 +109,11 @@ 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. >> + */ > > 1. Please, add a comment, that there is nothing special about > keeping the sequence. Only one collation could be stored, but > the present solution is simpler. diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c index 504096e6d..30b9bd9d6 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); >> 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 0bf54576d..3c5d3053a 100755 >> --- a/test/sql-tap/collation.test.lua >> +++ b/test/sql-tap/collation.test.lua >> @@ -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. Must pass. > > 2. Of course it must pass. It is the purpose of test. Please, > drop 'Must pass’. Removed. commit c409d222c500ef2b355d0c4061a3931aa6374780 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 quries: 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 504096e6d..30b9bd9d6 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 0bf54576d..9d0076e1d 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] 14+ messages in thread
* [tarantool-patches] [PATCH 2/2] sql: allow <COLLATE> only for string-like args 2019-05-08 11:29 [tarantool-patches] [PATCH 0/2] sql: allow <COLLATE> only for string-like args Roman Khabibov 2019-05-08 11:29 ` [tarantool-patches] [PATCH 1/2] sql: fix collation node duplication in AST Roman Khabibov @ 2019-05-08 11:29 ` Roman Khabibov 2019-05-12 16:32 ` [tarantool-patches] " Vladislav Shpilevoy 1 sibling, 1 reply; 14+ messages in thread From: Roman Khabibov @ 2019-05-08 11:29 UTC (permalink / raw) To: tarantool-patches; +Cc: v.shpilevoy Before this patch, user could use COLLATE with non-string-like literals, columns or subquery results. Disallow such usage. Closes #3804 --- src/box/sql/expr.c | 16 +++++++++ test/sql-tap/collation.test.lua | 50 ++++++++++++++++++++++++++- test/sql-tap/distinct.test.lua | 8 ++--- test/sql-tap/e_select1.test.lua | 2 +- test/sql-tap/identifier_case.test.lua | 2 +- test/sql-tap/resolver01.test.lua | 12 +++---- test/sql-tap/select1.test.lua | 2 +- test/sql-tap/selectE.test.lua | 15 +++----- test/sql-tap/with1.test.lua | 2 +- 9 files changed, 84 insertions(+), 25 deletions(-) diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index ba7eea59d..29e3386fa 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -4215,6 +4215,22 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) } case TK_SPAN: case TK_COLLATE:{ + enum field_type type; + struct Expr *left = pExpr->pLeft; + if (left->op == TK_COLUMN) { + int col_num = left->iColumn; + type = left->space_def->fields[col_num].type; + } else + type = left->type; + if (left->op != TK_CONCAT && + type != FIELD_TYPE_STRING && + type != FIELD_TYPE_SCALAR) { + diag_set(ClientError, ER_SQL_PARSER_GENERIC, + "COLLATE can't be used with " + "non-string arguments"); + pParse->is_aborted = true; + break; + } return sqlExprCodeTarget(pParse, pExpr->pLeft, target); } diff --git a/test/sql-tap/collation.test.lua b/test/sql-tap/collation.test.lua index 0bf54576d..352bede64 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(183) local prefix = "collation-" @@ -529,4 +529,52 @@ 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.6.0", + [[ CREATE TABLE test1 (one INT PRIMARY KEY, two INT) ]], + {}) + +test:do_catchsql_test( + "collation-2.6.1", + 'SELECT one COLLATE BINARY FROM test1', + {1, "COLLATE can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.6.2", + 'SELECT one COLLATE "unicode_ci" FROM test1', + {1, "COLLATE can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.6.3", + 'SELECT two COLLATE BINARY FROM test1', + {1, "COLLATE can't be used with non-string arguments"}) + + +test:do_catchsql_test( + "collation-2.6.4", + 'SELECT (one + two) COLLATE BINARY FROM test1', + {1, "COLLATE can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.6.5", + 'SELECT (SELECT one FROM test1) COLLATE BINARY', + {1, "COLLATE can't be used with non-string arguments"}) + +test:do_execsql_test( + "collation-2.6.6", + 'SELECT TRIM(\'A\') COLLATE BINARY', + {"A"}) + +test:do_catchsql_test( + "collation-2.6.7", + 'SELECT RANDOM() COLLATE BINARY', + {1, "COLLATE can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.6.8", + 'SELECT LENGTH(\'A\') COLLATE BINARY', + {1, "COLLATE can't be used with non-string arguments"}) + test:finish_test() diff --git a/test/sql-tap/distinct.test.lua b/test/sql-tap/distinct.test.lua index e6970084e..43f4232a1 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,10 +173,10 @@ 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"}}, + {"b COLLATE \"unicode_ci\" FROM t1 ORDER BY b COLLATE \"unicode_ci\"", {}, {"B"}}, } for tn, val in ipairs(data) do local sql = val[1] diff --git a/test/sql-tap/e_select1.test.lua b/test/sql-tap/e_select1.test.lua index c4724e636..566f2e723 100755 --- a/test/sql-tap/e_select1.test.lua +++ b/test/sql-tap/e_select1.test.lua @@ -1938,7 +1938,7 @@ test:do_execsql_test( test:do_execsql_test( "e_select-8.9.2", [[ - SELECT x COLLATE "binary" FROM d4 ORDER BY 1 COLLATE "unicode_ci" + SELECT x COLLATE "unicode_ci" FROM d4 ORDER BY 1 ]], { -- <e_select-8.9.2> "abc", "DEF", "ghi", "JKL" 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/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/selectE.test.lua b/test/sql-tap/selectE.test.lua index 32c3d6a06..16075d38e 100755 --- a/test/sql-tap/selectE.test.lua +++ b/test/sql-tap/selectE.test.lua @@ -57,8 +57,7 @@ test:do_test( CREATE TABLE t3(a TEXT primary key); INSERT INTO t3 VALUES('def'),('jkl'); - SELECT a FROM t1 EXCEPT SELECT a FROM t2 - ORDER BY a COLLATE "unicode_ci"; + SELECT a FROM t1 EXCEPT SELECT a FROM t2 ORDER BY a COLLATE "unicode_ci"; ]] end, { -- <selectE-1.0> @@ -70,8 +69,7 @@ test:do_test( "selectE-1.1", function() return test:execsql [[ - SELECT a FROM t2 EXCEPT SELECT a FROM t3 - ORDER BY a COLLATE "unicode_ci"; + SELECT a FROM t2 EXCEPT SELECT a FROM t3 ORDER BY a COLLATE "unicode_ci"; ]] end, { -- <selectE-1.1> @@ -83,8 +81,7 @@ test:do_test( "selectE-1.2", function() return test:execsql [[ - SELECT a FROM t2 EXCEPT SELECT a FROM t3 - ORDER BY a COLLATE "binary"; + SELECT a FROM t2 EXCEPT SELECT a FROM t3 ORDER BY a COLLATE "binary"; ]] end, { -- <selectE-1.2> @@ -96,8 +93,7 @@ test:do_test( "selectE-1.3", function() return test:execsql [[ - SELECT a FROM t2 EXCEPT SELECT a FROM t3 - ORDER BY a; + SELECT a FROM t2 EXCEPT SELECT a FROM t3 ORDER BY a; ]] end, { -- <selectE-1.3> @@ -113,8 +109,7 @@ test:do_test( DELETE FROM t3; INSERT INTO t2 VALUES('ABC'),('def'),('GHI'),('jkl'); INSERT INTO t3 SELECT lower(a) FROM t2; - SELECT a COLLATE "unicode_ci" FROM t2 EXCEPT SELECT a FROM t3 - ORDER BY 1 + SELECT a COLLATE "unicode_ci" FROM t2 EXCEPT SELECT a FROM t3 ORDER BY 1 ]] end, { -- <selectE-2.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> -- 2.20.1 (Apple Git-117) ^ permalink raw reply [flat|nested] 14+ messages in thread
* [tarantool-patches] Re: [PATCH 2/2] sql: allow <COLLATE> only for string-like args 2019-05-08 11:29 ` [tarantool-patches] [PATCH 2/2] sql: allow <COLLATE> only for string-like args Roman Khabibov @ 2019-05-12 16:32 ` Vladislav Shpilevoy 2019-05-28 14:08 ` Roman Khabibov 0 siblings, 1 reply; 14+ messages in thread From: Vladislav Shpilevoy @ 2019-05-12 16:32 UTC (permalink / raw) To: Roman Khabibov, tarantool-patches Sorry, the patch does not work: tarantool> box.execute('SELECT NOT TRUE COLLATE "unicode"') --- - metadata: - name: NOT TRUE COLLATE "unicode" type: boolean rows: - [false] ... Obviously, 'NOT TRUE' is not a string. Strange, but without 'NOT' everything is fine: tarantool> box.execute('SELECT TRUE COLLATE "unicode"') --- - error: COLLATE can't be used with non-string arguments ... Please, fix. > diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c > index ba7eea59d..29e3386fa 100644 > --- a/src/box/sql/expr.c > +++ b/src/box/sql/expr.c > @@ -4215,6 +4215,22 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) > } > case TK_SPAN: > case TK_COLLATE:{ > + enum field_type type; > + struct Expr *left = pExpr->pLeft; > + if (left->op == TK_COLUMN) { > + int col_num = left->iColumn; > + type = left->space_def->fields[col_num].type; > + } else > + type = left->type; > + if (left->op != TK_CONCAT && Why do you check for TK_CONCAT? Its type should be FIELD_TYPE_STRING. > + type != FIELD_TYPE_STRING && > + type != FIELD_TYPE_SCALAR) { > + diag_set(ClientError, ER_SQL_PARSER_GENERIC, > + "COLLATE can't be used with " > + "non-string arguments"); > + pParse->is_aborted = true; > + break; > + } > return sqlExprCodeTarget(pParse, pExpr->pLeft, > target); > } ^ permalink raw reply [flat|nested] 14+ messages in thread
* [tarantool-patches] Re: [PATCH 2/2] sql: allow <COLLATE> only for string-like args 2019-05-12 16:32 ` [tarantool-patches] " Vladislav Shpilevoy @ 2019-05-28 14:08 ` Roman Khabibov 2019-06-02 17:09 ` Vladislav Shpilevoy 0 siblings, 1 reply; 14+ messages in thread From: Roman Khabibov @ 2019-05-28 14:08 UTC (permalink / raw) To: tarantool-patches; +Cc: Vladislav Shpilevoy Hi! Thanks for the review. > On May 12, 2019, at 7:32 PM, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote: > > Sorry, the patch does not work: > > tarantool> box.execute('SELECT NOT TRUE COLLATE "unicode"') > --- > - metadata: > - name: NOT TRUE COLLATE "unicode" > type: boolean > rows: > - [false] > ... > > Obviously, 'NOT TRUE' is not a string. Strange, but without 'NOT' > everything is fine: > > tarantool> box.execute('SELECT TRUE COLLATE "unicode"') > --- > - error: COLLATE can't be used with non-string arguments > ... > > Please, fix. Fixed. +test:do_catchsql_test( + "collation-2.7.9", + 'SELECT TRUE COLLATE \"unicode\"', + {1, "COLLATE can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.7.10", + 'SELECT NOT TRUE COLLATE \"unicode\"', + {1, "COLLATE can't be used with non-string arguments"}) + + >> diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c >> index ba7eea59d..29e3386fa 100644 >> --- a/src/box/sql/expr.c >> +++ b/src/box/sql/expr.c >> @@ -4215,6 +4215,22 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) >> } >> case TK_SPAN: >> case TK_COLLATE:{ >> + enum field_type type; >> + struct Expr *left = pExpr->pLeft; >> + if (left->op == TK_COLUMN) { >> + int col_num = left->iColumn; >> + type = left->space_def->fields[col_num].type; >> + } else >> + type = left->type; >> + if (left->op != TK_CONCAT && > > Why do you check for TK_CONCAT? Its type should be FIELD_TYPE_STRING. Concatenations are rejected without this check. commit 5feaa9330f2c642fc16b479383b1c5cac96c28cc 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 ba7eea59d..e558cbb18 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 @expr with the collation in the root + * can be used with <COLLATE>. If it is not, leave an error + * message in pParse. + * + * @param pParse Parser context. + * @param expr Expression for checking. + * + * @retval 0 on success. + * @retval -1 on error. + */ +static int +check_collate_arg(Parse *parse, 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 (left->op != TK_CONCAT && type != FIELD_TYPE_STRING && + type != FIELD_TYPE_SCALAR) { + diag_set(ClientError, ER_SQL_PARSER_GENERIC, + "COLLATE 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) @@ -3935,6 +3969,9 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) testcase(op == TK_BITNOT); assert(TK_NOT == OP_Not); testcase(op == TK_NOT); + if (pExpr->pLeft->op == TK_COLLATE && + check_collate_arg(pParse, pExpr->pLeft) != 0) + break; r1 = sqlExprCodeTemp(pParse, pExpr->pLeft, ®Free1); testcase(regFree1 == 0); @@ -4215,6 +4252,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); } @@ -4439,6 +4478,8 @@ int sqlExprCodeTemp(Parse * pParse, Expr * pExpr, int *pReg) { int r2; + if (pExpr->op == TK_COLLATE && check_collate_arg(pParse, pExpr) != 0) + return -1; 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 3c5d3053a..4d4329717 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(188) local prefix = "collation-" @@ -546,4 +546,63 @@ 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 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 can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.7.3", + 'SELECT two COLLATE BINARY FROM test1', + {1, "COLLATE 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 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 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 can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.7.8", + 'SELECT LENGTH(\'A\') COLLATE BINARY', + {1, "COLLATE can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.7.9", + 'SELECT TRUE COLLATE \"unicode\"', + {1, "COLLATE can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.7.10", + 'SELECT NOT TRUE COLLATE \"unicode\"', + {1, "COLLATE can't be used with non-string arguments"}) + + 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/e_select1.test.lua b/test/sql-tap/e_select1.test.lua index c4724e636..566f2e723 100755 --- a/test/sql-tap/e_select1.test.lua +++ b/test/sql-tap/e_select1.test.lua @@ -1938,7 +1938,7 @@ test:do_execsql_test( test:do_execsql_test( "e_select-8.9.2", [[ - SELECT x COLLATE "binary" FROM d4 ORDER BY 1 COLLATE "unicode_ci" + SELECT x COLLATE "unicode_ci" FROM d4 ORDER BY 1 ]], { -- <e_select-8.9.2> "abc", "DEF", "ghi", "JKL" 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/selectE.test.lua b/test/sql-tap/selectE.test.lua index 32c3d6a06..16075d38e 100755 --- a/test/sql-tap/selectE.test.lua +++ b/test/sql-tap/selectE.test.lua @@ -57,8 +57,7 @@ test:do_test( CREATE TABLE t3(a TEXT primary key); INSERT INTO t3 VALUES('def'),('jkl'); - SELECT a FROM t1 EXCEPT SELECT a FROM t2 - ORDER BY a COLLATE "unicode_ci"; + SELECT a FROM t1 EXCEPT SELECT a FROM t2 ORDER BY a COLLATE "unicode_ci"; ]] end, { -- <selectE-1.0> @@ -70,8 +69,7 @@ test:do_test( "selectE-1.1", function() return test:execsql [[ - SELECT a FROM t2 EXCEPT SELECT a FROM t3 - ORDER BY a COLLATE "unicode_ci"; + SELECT a FROM t2 EXCEPT SELECT a FROM t3 ORDER BY a COLLATE "unicode_ci"; ]] end, { -- <selectE-1.1> @@ -83,8 +81,7 @@ test:do_test( "selectE-1.2", function() return test:execsql [[ - SELECT a FROM t2 EXCEPT SELECT a FROM t3 - ORDER BY a COLLATE "binary"; + SELECT a FROM t2 EXCEPT SELECT a FROM t3 ORDER BY a COLLATE "binary"; ]] end, { -- <selectE-1.2> @@ -96,8 +93,7 @@ test:do_test( "selectE-1.3", function() return test:execsql [[ - SELECT a FROM t2 EXCEPT SELECT a FROM t3 - ORDER BY a; + SELECT a FROM t2 EXCEPT SELECT a FROM t3 ORDER BY a; ]] end, { -- <selectE-1.3> @@ -113,8 +109,7 @@ test:do_test( DELETE FROM t3; INSERT INTO t2 VALUES('ABC'),('def'),('GHI'),('jkl'); INSERT INTO t3 SELECT lower(a) FROM t2; - SELECT a COLLATE "unicode_ci" FROM t2 EXCEPT SELECT a FROM t3 - ORDER BY 1 + SELECT a COLLATE "unicode_ci" FROM t2 EXCEPT SELECT a FROM t3 ORDER BY 1 ]] end, { -- <selectE-2.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] 14+ messages in thread
* [tarantool-patches] Re: [PATCH 2/2] sql: allow <COLLATE> only for string-like args 2019-05-28 14:08 ` Roman Khabibov @ 2019-06-02 17:09 ` Vladislav Shpilevoy 2019-06-04 14:27 ` Roman Khabibov 0 siblings, 1 reply; 14+ messages in thread From: Vladislav Shpilevoy @ 2019-06-02 17:09 UTC (permalink / raw) To: Roman Khabibov, tarantool-patches Thanks for the fixes! See 8 comments below. >>> diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c >>> index ba7eea59d..29e3386fa 100644 >>> --- a/src/box/sql/expr.c >>> +++ b/src/box/sql/expr.c >>> @@ -4215,6 +4215,22 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) >>> } >>> case TK_SPAN: >>> case TK_COLLATE:{ >>> + enum field_type type; >>> + struct Expr *left = pExpr->pLeft; >>> + if (left->op == TK_COLUMN) { >>> + int col_num = left->iColumn; >>> + type = left->space_def->fields[col_num].type; >>> + } else >>> + type = left->type; >>> + if (left->op != TK_CONCAT && >> >> Why do you check for TK_CONCAT? Its type should be FIELD_TYPE_STRING. > Concatenations are rejected without this check. 1. It is not a good answer, when you can't explain, why you need a line of code, but a program does not work without it. Please, investigate and explain, why do you check for both 'type == STRING' and 'op == CONCAT'. If an op is CONCAT, then type is already STRING, so the additional check for 'op' should not be necessary. > > commit 5feaa9330f2c642fc16b479383b1c5cac96c28cc > 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 ba7eea59d..e558cbb18 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 @expr with the collation in the root 2. '@expr' is an invalid Doxygen command. If you want to reference a parameter, use <@a parameter_name>. See Doxygen documentation. > + * can be used with <COLLATE>. If it is not, leave an error > + * message in pParse. 3. There is no argument 'pParse'. > + * > + * @param pParse Parser context. > + * @param expr Expression for checking. > + * > + * @retval 0 on success. > + * @retval -1 on error. > + */ > +static int > +check_collate_arg(Parse *parse, Expr *expr) 4. We use 'struct' prefix in all new SQL code. > +{ > + 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; 5. We always either use '{}' for both 'if-else' parts, or do not use it at all. > + if (left->op != TK_CONCAT && type != FIELD_TYPE_STRING && > + type != FIELD_TYPE_SCALAR) { > + diag_set(ClientError, ER_SQL_PARSER_GENERIC, > + "COLLATE can't be used with " > + "non-string arguments"); > + parse->is_aborted = true; > + return -1; > + } > + return 0; > @@ -3935,6 +3969,9 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) > testcase(op == TK_BITNOT); > assert(TK_NOT == OP_Not); > testcase(op == TK_NOT); > + if (pExpr->pLeft->op == TK_COLLATE && > + check_collate_arg(pParse, pExpr->pLeft) != 0) > + break; > r1 = sqlExprCodeTemp(pParse, pExpr->pLeft, > ®Free1); 6. Why do you check for COLLATE before CodeTemp()? This function does exactly the same check. > testcase(regFree1 == 0); > diff --git a/test/sql-tap/e_select1.test.lua b/test/sql-tap/e_select1.test.lua > index c4724e636..566f2e723 100755 > --- a/test/sql-tap/e_select1.test.lua > +++ b/test/sql-tap/e_select1.test.lua > @@ -1938,7 +1938,7 @@ test:do_execsql_test( > test:do_execsql_test( > "e_select-8.9.2", > [[ > - SELECT x COLLATE "binary" FROM d4 ORDER BY 1 COLLATE "unicode_ci" > + SELECT x COLLATE "unicode_ci" FROM d4 ORDER BY 1 7. Why? The old test should pass as well. '1' here is not a value, but an index of a selected column. > ]], { > -- <e_select-8.9.2> > "abc", "DEF", "ghi", "JKL" > diff --git a/test/sql-tap/selectE.test.lua b/test/sql-tap/selectE.test.lua > index 32c3d6a06..16075d38e 100755 > --- a/test/sql-tap/selectE.test.lua > +++ b/test/sql-tap/selectE.test.lua > @@ -57,8 +57,7 @@ test:do_test( > CREATE TABLE t3(a TEXT primary key); > INSERT INTO t3 VALUES('def'),('jkl'); > > - SELECT a FROM t1 EXCEPT SELECT a FROM t2 > - ORDER BY a COLLATE "unicode_ci"; > + SELECT a FROM t1 EXCEPT SELECT a FROM t2 ORDER BY a COLLATE "unicode_ci"; > ]] > end, { > -- <selectE-1.0> > @@ -70,8 +69,7 @@ test:do_test( > "selectE-1.1", > function() > return test:execsql [[ > - SELECT a FROM t2 EXCEPT SELECT a FROM t3 > - ORDER BY a COLLATE "unicode_ci"; > + SELECT a FROM t2 EXCEPT SELECT a FROM t3 ORDER BY a COLLATE "unicode_ci"; > ]] > end, { > -- <selectE-1.1> > @@ -83,8 +81,7 @@ test:do_test( > "selectE-1.2", > function() > return test:execsql [[ > - SELECT a FROM t2 EXCEPT SELECT a FROM t3 > - ORDER BY a COLLATE "binary"; > + SELECT a FROM t2 EXCEPT SELECT a FROM t3 ORDER BY a COLLATE "binary"; > ]] > end, { > -- <selectE-1.2> > @@ -96,8 +93,7 @@ test:do_test( > "selectE-1.3", > function() > return test:execsql [[ > - SELECT a FROM t2 EXCEPT SELECT a FROM t3 > - ORDER BY a; > + SELECT a FROM t2 EXCEPT SELECT a FROM t3 ORDER BY a; > ]] > end, { > -- <selectE-1.3> > @@ -113,8 +109,7 @@ test:do_test( > DELETE FROM t3; > INSERT INTO t2 VALUES('ABC'),('def'),('GHI'),('jkl'); > INSERT INTO t3 SELECT lower(a) FROM t2; > - SELECT a COLLATE "unicode_ci" FROM t2 EXCEPT SELECT a FROM t3 > - ORDER BY 1 > + SELECT a COLLATE "unicode_ci" FROM t2 EXCEPT SELECT a FROM t3 ORDER BY 1 > ]] > end, { 8. Why do you need changes in this file? You just made multiline requests one line. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [tarantool-patches] Re: [PATCH 2/2] sql: allow <COLLATE> only for string-like args 2019-06-02 17:09 ` Vladislav Shpilevoy @ 2019-06-04 14:27 ` Roman Khabibov 2019-06-04 19:49 ` Vladislav Shpilevoy 0 siblings, 1 reply; 14+ messages in thread From: Roman Khabibov @ 2019-06-04 14:27 UTC (permalink / raw) To: tarantool-patches; +Cc: Vladislav Shpilevoy Hello, thanks for the review. > On Jun 2, 2019, at 8:09 PM, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote: > > Thanks for the fixes! See 8 comments below. > >>>> diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c >>>> index ba7eea59d..29e3386fa 100644 >>>> --- a/src/box/sql/expr.c >>>> +++ b/src/box/sql/expr.c >>>> @@ -4215,6 +4215,22 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) >>>> } >>>> case TK_SPAN: >>>> case TK_COLLATE:{ >>>> + enum field_type type; >>>> + struct Expr *left = pExpr->pLeft; >>>> + if (left->op == TK_COLUMN) { >>>> + int col_num = left->iColumn; >>>> + type = left->space_def->fields[col_num].type; >>>> + } else >>>> + type = left->type; >>>> + if (left->op != TK_CONCAT && >>> >>> Why do you check for TK_CONCAT? Its type should be FIELD_TYPE_STRING. >> Concatenations are rejected without this check. > > 1. It is not a good answer, when you can't explain, why you need > a line of code, but a program does not work without it. Please, investigate > and explain, why do you check for both 'type == STRING' and 'op == CONCAT'. > If an op is CONCAT, then type is already STRING, so the additional check > for 'op' should not be necessary. @@ -1060,6 +1094,8 @@ sqlPExpr(Parse * pParse, /* Parsing context */ if (p) { memset(p, 0, sizeof(Expr)); p->op = op & TKFLG_MASK; + if (op == TK_CONCAT) + p->type = FIELD_TYPE_STRING; p->iAgg = -1; } sqlExprAttachSubtrees(pParse->db, p, pLeft, pRight); >> >> commit 5feaa9330f2c642fc16b479383b1c5cac96c28cc >> 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 ba7eea59d..e558cbb18 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 @expr with the collation in the root > > 2. '@expr' is an invalid Doxygen command. If you want to reference > a parameter, use <@a parameter_name>. See Doxygen documentation. > >> + * can be used with <COLLATE>. If it is not, leave an error >> + * message in pParse. > > 3. There is no argument 'pParse'. > >> + * >> + * @param pParse Parser context. >> + * @param expr Expression for checking. >> + * >> + * @retval 0 on success. >> + * @retval -1 on error. >> + */ >> +static int >> +check_collate_arg(Parse *parse, Expr *expr) > > 4. We use 'struct' prefix in all new SQL code. > >> +{ >> + 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; > > 5. We always either use '{}' for both 'if-else' parts, > or do not use it at all. diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index ba7eea59d..d5bc733c3 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"); + parse->is_aborted = true; + return -1; + } + return 0; +} + >> + if (left->op != TK_CONCAT && type != FIELD_TYPE_STRING && >> + type != FIELD_TYPE_SCALAR) { >> + diag_set(ClientError, ER_SQL_PARSER_GENERIC, >> + "COLLATE can't be used with " >> + "non-string arguments"); >> + parse->is_aborted = true; >> + return -1; >> + } >> + return 0; >> @@ -3935,6 +3969,9 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) >> testcase(op == TK_BITNOT); >> assert(TK_NOT == OP_Not); >> testcase(op == TK_NOT); >> + if (pExpr->pLeft->op == TK_COLLATE && >> + check_collate_arg(pParse, pExpr->pLeft) != 0) >> + break; >> r1 = sqlExprCodeTemp(pParse, pExpr->pLeft, >> ®Free1); > > 6. Why do you check for COLLATE before CodeTemp()? This function > does exactly the same check. Forgot to remove it. >> testcase(regFree1 == 0); >> diff --git a/test/sql-tap/e_select1.test.lua b/test/sql-tap/e_select1.test.lua >> index c4724e636..566f2e723 100755 >> --- a/test/sql-tap/e_select1.test.lua >> +++ b/test/sql-tap/e_select1.test.lua >> @@ -1938,7 +1938,7 @@ test:do_execsql_test( >> test:do_execsql_test( >> "e_select-8.9.2", >> [[ >> - SELECT x COLLATE "binary" FROM d4 ORDER BY 1 COLLATE "unicode_ci" >> + SELECT x COLLATE "unicode_ci" FROM d4 ORDER BY 1 > > 7. Why? The old test should pass as well. '1' here is not a value, but an > index of a selected column. Removed. >> ]], { >> -- <e_select-8.9.2> >> "abc", "DEF", "ghi", "JKL" >> diff --git a/test/sql-tap/selectE.test.lua b/test/sql-tap/selectE.test.lua >> index 32c3d6a06..16075d38e 100755 >> --- a/test/sql-tap/selectE.test.lua >> +++ b/test/sql-tap/selectE.test.lua >> @@ -57,8 +57,7 @@ test:do_test( >> CREATE TABLE t3(a TEXT primary key); >> INSERT INTO t3 VALUES('def'),('jkl'); >> >> - SELECT a FROM t1 EXCEPT SELECT a FROM t2 >> - ORDER BY a COLLATE "unicode_ci"; >> + SELECT a FROM t1 EXCEPT SELECT a FROM t2 ORDER BY a COLLATE "unicode_ci"; >> ]] >> end, { >> -- <selectE-1.0> >> @@ -70,8 +69,7 @@ test:do_test( >> "selectE-1.1", >> function() >> return test:execsql [[ >> - SELECT a FROM t2 EXCEPT SELECT a FROM t3 >> - ORDER BY a COLLATE "unicode_ci"; >> + SELECT a FROM t2 EXCEPT SELECT a FROM t3 ORDER BY a COLLATE "unicode_ci"; >> ]] >> end, { >> -- <selectE-1.1> >> @@ -83,8 +81,7 @@ test:do_test( >> "selectE-1.2", >> function() >> return test:execsql [[ >> - SELECT a FROM t2 EXCEPT SELECT a FROM t3 >> - ORDER BY a COLLATE "binary"; >> + SELECT a FROM t2 EXCEPT SELECT a FROM t3 ORDER BY a COLLATE "binary"; >> ]] >> end, { >> -- <selectE-1.2> >> @@ -96,8 +93,7 @@ test:do_test( >> "selectE-1.3", >> function() >> return test:execsql [[ >> - SELECT a FROM t2 EXCEPT SELECT a FROM t3 >> - ORDER BY a; >> + SELECT a FROM t2 EXCEPT SELECT a FROM t3 ORDER BY a; >> ]] >> end, { >> -- <selectE-1.3> >> @@ -113,8 +109,7 @@ test:do_test( >> DELETE FROM t3; >> INSERT INTO t2 VALUES('ABC'),('def'),('GHI'),('jkl'); >> INSERT INTO t3 SELECT lower(a) FROM t2; >> - SELECT a COLLATE "unicode_ci" FROM t2 EXCEPT SELECT a FROM t3 >> - ORDER BY 1 >> + SELECT a COLLATE "unicode_ci" FROM t2 EXCEPT SELECT a FROM t3 ORDER BY 1 >> ]] >> end, { > > 8. Why do you need changes in this file? You just made multiline requests > one line. Removed. commit 879f3c6716a7414247ff4902bdc9117531c47ef6 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 ba7eea59d..d5bc733c3 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"); + 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) @@ -1060,6 +1094,8 @@ sqlPExpr(Parse * pParse, /* Parsing context */ if (p) { memset(p, 0, sizeof(Expr)); p->op = op & TKFLG_MASK; + if (op == TK_CONCAT) + p->type = FIELD_TYPE_STRING; p->iAgg = -1; } sqlExprAttachSubtrees(pParse->db, p, pLeft, pRight); @@ -3935,6 +3971,9 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) testcase(op == TK_BITNOT); assert(TK_NOT == OP_Not); testcase(op == TK_NOT); + if (pExpr->pLeft->op == TK_COLLATE && + check_collate_arg(pParse, pExpr->pLeft) != 0) + break; r1 = sqlExprCodeTemp(pParse, pExpr->pLeft, ®Free1); testcase(regFree1 == 0); @@ -4215,6 +4254,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/collation.test.lua b/test/sql-tap/collation.test.lua index 9d0076e1d..dc2a0b8ef 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(188) local prefix = "collation-" @@ -546,4 +546,63 @@ 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 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 can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.7.3", + 'SELECT two COLLATE BINARY FROM test1', + {1, "COLLATE 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 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 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 can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.7.8", + 'SELECT LENGTH(\'A\') COLLATE BINARY', + {1, "COLLATE can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.7.9", + 'SELECT TRUE COLLATE \"unicode\"', + {1, "COLLATE can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.7.10", + 'SELECT NOT TRUE COLLATE \"unicode\"', + {1, "COLLATE can't be used with non-string arguments"}) + + 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/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] 14+ messages in thread
* [tarantool-patches] Re: [PATCH 2/2] sql: allow <COLLATE> only for string-like args 2019-06-04 14:27 ` Roman Khabibov @ 2019-06-04 19:49 ` Vladislav Shpilevoy 2019-06-07 15:01 ` Roman Khabibov 0 siblings, 1 reply; 14+ messages in thread From: Vladislav Shpilevoy @ 2019-06-04 19:49 UTC (permalink / raw) To: Roman Khabibov, tarantool-patches, n.pettik Hi! tarantool> box.execute('SELECT TRUE AND TRUE COLLATE "binary";') --- - metadata: - name: TRUE AND TRUE COLLATE "binary" type: boolean rows: - [true] ... Your previous solution was correct, you should check collation in CodeTemp. But somewhy you decided to remove it and keep only in 'TK_NOT' and 'TK_COLLATE'. Obviously, you can't add the check to each 'case' in that 'switch', and you need to check the collation for every token. Besides, Nikita, please, help us with the next comment. On 04/06/2019 17:27, Roman Khabibov wrote: > Hello, thanks for the review. > >> On Jun 2, 2019, at 8:09 PM, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote: >> >> Thanks for the fixes! See 8 comments below. >> >>>>> diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c >>>>> index ba7eea59d..29e3386fa 100644 >>>>> --- a/src/box/sql/expr.c >>>>> +++ b/src/box/sql/expr.c >>>>> @@ -4215,6 +4215,22 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) >>>>> } >>>>> case TK_SPAN: >>>>> case TK_COLLATE:{ >>>>> + enum field_type type; >>>>> + struct Expr *left = pExpr->pLeft; >>>>> + if (left->op == TK_COLUMN) { >>>>> + int col_num = left->iColumn; >>>>> + type = left->space_def->fields[col_num].type; >>>>> + } else >>>>> + type = left->type; >>>>> + if (left->op != TK_CONCAT && >>>> >>>> Why do you check for TK_CONCAT? Its type should be FIELD_TYPE_STRING. >>> Concatenations are rejected without this check. >> >> 1. It is not a good answer, when you can't explain, why you need >> a line of code, but a program does not work without it. Please, investigate >> and explain, why do you check for both 'type == STRING' and 'op == CONCAT'. >> If an op is CONCAT, then type is already STRING, so the additional check >> for 'op' should not be necessary. > @@ -1060,6 +1094,8 @@ sqlPExpr(Parse * pParse, /* Parsing context */ > if (p) { > memset(p, 0, sizeof(Expr)); > p->op = op & TKFLG_MASK; > + if (op == TK_CONCAT) > + p->type = FIELD_TYPE_STRING; > p->iAgg = -1; > } > sqlExprAttachSubtrees(pParse->db, p, pLeft, pRight); > I am 100% sure, that it is not a place for types setting. This function, and its comment says, only allocates. For complex types we have sql_expr_type(), we can use Expr.type for basic types located in tree lists only. Nikita knows more about typing, he can help better. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [tarantool-patches] Re: [PATCH 2/2] sql: allow <COLLATE> only for string-like args 2019-06-04 19:49 ` Vladislav Shpilevoy @ 2019-06-07 15:01 ` Roman Khabibov 2019-06-09 16:55 ` Vladislav Shpilevoy 0 siblings, 1 reply; 14+ messages in thread From: Roman Khabibov @ 2019-06-07 15:01 UTC (permalink / raw) To: tarantool-patches; +Cc: Vladislav Shpilevoy Hi! Thanks for the review. > On Jun 4, 2019, at 10:49 PM, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote: > > Hi! > > tarantool> box.execute('SELECT TRUE AND TRUE COLLATE "binary";') > --- > - metadata: > - name: TRUE AND TRUE COLLATE "binary" > type: boolean > rows: > - [true] > ... > > Your previous solution was correct, you should check collation > in CodeTemp. But somewhy you decided to remove it and keep only > in 'TK_NOT' and 'TK_COLLATE'. Obviously, you can't add the check > to each 'case' in that 'switch', and you need to check the collation > for every token. @ -4438,6 +4475,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) > Besides, Nikita, please, help us with the next comment. > On 04/06/2019 17:27, Roman Khabibov wrote: >> Hello, thanks for the review. >> >>> On Jun 2, 2019, at 8:09 PM, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote: >>> >>> Thanks for the fixes! See 8 comments below. >>> >>>>>> diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c >>>>>> index ba7eea59d..29e3386fa 100644 >>>>>> --- a/src/box/sql/expr.c >>>>>> +++ b/src/box/sql/expr.c >>>>>> @@ -4215,6 +4215,22 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) >>>>>> } >>>>>> case TK_SPAN: >>>>>> case TK_COLLATE:{ >>>>>> + enum field_type type; >>>>>> + struct Expr *left = pExpr->pLeft; >>>>>> + if (left->op == TK_COLUMN) { >>>>>> + int col_num = left->iColumn; >>>>>> + type = left->space_def->fields[col_num].type; >>>>>> + } else >>>>>> + type = left->type; >>>>>> + if (left->op != TK_CONCAT && >>>>> >>>>> Why do you check for TK_CONCAT? Its type should be FIELD_TYPE_STRING. >>>> Concatenations are rejected without this check. >>> >>> 1. It is not a good answer, when you can't explain, why you need >>> a line of code, but a program does not work without it. Please, investigate >>> and explain, why do you check for both 'type == STRING' and 'op == CONCAT'. >>> If an op is CONCAT, then type is already STRING, so the additional check >>> for 'op' should not be necessary. >> @@ -1060,6 +1094,8 @@ sqlPExpr(Parse * pParse, /* Parsing context */ >> if (p) { >> memset(p, 0, sizeof(Expr)); >> p->op = op & TKFLG_MASK; >> + if (op == TK_CONCAT) >> + p->type = FIELD_TYPE_STRING; >> p->iAgg = -1; >> } >> sqlExprAttachSubtrees(pParse->db, p, pLeft, pRight); >> > > I am 100% sure, that it is not a place for types setting. This > function, and its comment says, only allocates. For complex types > we have sql_expr_type(), we can use Expr.type for basic types > located in tree lists only. > > Nikita knows more about typing, he can help better. @@ -4215,6 +4249,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); } commit 1cceb4ffa3a8b5c513ea742bb243ae963bc78ea9 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 ba7eea59d..bc6c1dcf3 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"); + 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) @@ -4215,6 +4249,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); } @@ -4438,6 +4475,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 9d0076e1d..a15cf0756 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', + {1, "COLLATE 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 can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.7.3", + 'SELECT two COLLATE BINARY FROM test1', + {1, "COLLATE 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 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 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 can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.7.8", + 'SELECT LENGTH(\'A\') COLLATE BINARY', + {1, "COLLATE can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.7.9", + 'SELECT TRUE COLLATE \"unicode\"', + {1, "COLLATE can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.7.10", + 'SELECT NOT TRUE COLLATE \"unicode\"', + {1, "COLLATE can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.7.11", + 'SELECT TRUE AND TRUE COLLATE \"unicode\"', + {1, "COLLATE can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.7.12", + 'SELECT 1 | 1 COLLATE \"unicode\"', + {1, "COLLATE can't be used with non-string arguments"}) + +test:do_catchsql_test( + "collation-2.7.14", + 'SELECT +\'str\' COLLATE \"unicode\"', + {0,{"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 ea684a73d..6740cd03a 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] 14+ messages in thread
* [tarantool-patches] Re: [PATCH 2/2] sql: allow <COLLATE> only for string-like args 2019-06-07 15:01 ` Roman Khabibov @ 2019-06-09 16:55 ` Vladislav Shpilevoy 0 siblings, 0 replies; 14+ messages in thread From: Vladislav Shpilevoy @ 2019-06-09 16:55 UTC (permalink / raw) To: Roman Khabibov, tarantool-patches, n.pettik Hi! Thanks for the fixes! Recently Nikita was busy with Expr.type assignment removals, and now you add another one. Also, inside check_collate_arg() you still use Expr.type explicitly, without sql_expr_type() function. This leads to another bug. This works: box.execute("SELECT (SELECT 'str') COLLATE binary;") This does not work: box.execute("SELECT (SELECT 'str') COLLATE binary COLLATE binary;") Obviously, it is not ok. Nikita, please, do a final proper review about what how to work with Expr types. On 07/06/2019 18:01, Roman Khabibov wrote: > Hi! Thanks for the review. > >> On Jun 4, 2019, at 10:49 PM, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote: >> >> Hi! >> >> tarantool> box.execute('SELECT TRUE AND TRUE COLLATE "binary";') >> --- >> - metadata: >> - name: TRUE AND TRUE COLLATE "binary" >> type: boolean >> rows: >> - [true] >> ... >> >> Your previous solution was correct, you should check collation >> in CodeTemp. But somewhy you decided to remove it and keep only >> in 'TK_NOT' and 'TK_COLLATE'. Obviously, you can't add the check >> to each 'case' in that 'switch', and you need to check the collation >> for every token. > @ -4438,6 +4475,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) > >> Besides, Nikita, please, help us with the next comment. >> On 04/06/2019 17:27, Roman Khabibov wrote: >>> Hello, thanks for the review. >>> >>>> On Jun 2, 2019, at 8:09 PM, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote: >>>> >>>> Thanks for the fixes! See 8 comments below. >>>> >>>>>>> diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c >>>>>>> index ba7eea59d..29e3386fa 100644 >>>>>>> --- a/src/box/sql/expr.c >>>>>>> +++ b/src/box/sql/expr.c >>>>>>> @@ -4215,6 +4215,22 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) >>>>>>> } >>>>>>> case TK_SPAN: >>>>>>> case TK_COLLATE:{ >>>>>>> + enum field_type type; >>>>>>> + struct Expr *left = pExpr->pLeft; >>>>>>> + if (left->op == TK_COLUMN) { >>>>>>> + int col_num = left->iColumn; >>>>>>> + type = left->space_def->fields[col_num].type; >>>>>>> + } else >>>>>>> + type = left->type; >>>>>>> + if (left->op != TK_CONCAT && >>>>>> >>>>>> Why do you check for TK_CONCAT? Its type should be FIELD_TYPE_STRING. >>>>> Concatenations are rejected without this check. >>>> >>>> 1. It is not a good answer, when you can't explain, why you need >>>> a line of code, but a program does not work without it. Please, investigate >>>> and explain, why do you check for both 'type == STRING' and 'op == CONCAT'. >>>> If an op is CONCAT, then type is already STRING, so the additional check >>>> for 'op' should not be necessary. >>> @@ -1060,6 +1094,8 @@ sqlPExpr(Parse * pParse, /* Parsing context */ >>> if (p) { >>> memset(p, 0, sizeof(Expr)); >>> p->op = op & TKFLG_MASK; >>> + if (op == TK_CONCAT) >>> + p->type = FIELD_TYPE_STRING; >>> p->iAgg = -1; >>> } >>> sqlExprAttachSubtrees(pParse->db, p, pLeft, pRight); >>> >> >> I am 100% sure, that it is not a place for types setting. This >> function, and its comment says, only allocates. For complex types >> we have sql_expr_type(), we can use Expr.type for basic types >> located in tree lists only. >> >> Nikita knows more about typing, he can help better. > @@ -4215,6 +4249,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); > } > > commit 1cceb4ffa3a8b5c513ea742bb243ae963bc78ea9 > 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 ba7eea59d..bc6c1dcf3 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"); > + 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) > @@ -4215,6 +4249,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); > } > @@ -4438,6 +4475,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 9d0076e1d..a15cf0756 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', > + {1, "COLLATE 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 can't be used with non-string arguments"}) > + > +test:do_catchsql_test( > + "collation-2.7.3", > + 'SELECT two COLLATE BINARY FROM test1', > + {1, "COLLATE 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 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 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 can't be used with non-string arguments"}) > + > +test:do_catchsql_test( > + "collation-2.7.8", > + 'SELECT LENGTH(\'A\') COLLATE BINARY', > + {1, "COLLATE can't be used with non-string arguments"}) > + > +test:do_catchsql_test( > + "collation-2.7.9", > + 'SELECT TRUE COLLATE \"unicode\"', > + {1, "COLLATE can't be used with non-string arguments"}) > + > +test:do_catchsql_test( > + "collation-2.7.10", > + 'SELECT NOT TRUE COLLATE \"unicode\"', > + {1, "COLLATE can't be used with non-string arguments"}) > + > +test:do_catchsql_test( > + "collation-2.7.11", > + 'SELECT TRUE AND TRUE COLLATE \"unicode\"', > + {1, "COLLATE can't be used with non-string arguments"}) > + > +test:do_catchsql_test( > + "collation-2.7.12", > + 'SELECT 1 | 1 COLLATE \"unicode\"', > + {1, "COLLATE can't be used with non-string arguments"}) > + > +test:do_catchsql_test( > + "collation-2.7.14", > + 'SELECT +\'str\' COLLATE \"unicode\"', > + {0,{"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 ea684a73d..6740cd03a 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] 14+ messages in thread
end of thread, other threads:[~2019-06-09 16:55 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-05-08 11:29 [tarantool-patches] [PATCH 0/2] sql: allow <COLLATE> only for string-like args Roman Khabibov 2019-05-08 11:29 ` [tarantool-patches] [PATCH 1/2] sql: fix collation node duplication in AST Roman Khabibov 2019-05-12 16:32 ` [tarantool-patches] " Vladislav Shpilevoy 2019-05-28 14:10 ` Roman Khabibov 2019-06-02 17:09 ` Vladislav Shpilevoy 2019-06-04 14:00 ` Roman Khabibov 2019-05-08 11:29 ` [tarantool-patches] [PATCH 2/2] sql: allow <COLLATE> only for string-like args Roman Khabibov 2019-05-12 16:32 ` [tarantool-patches] " Vladislav Shpilevoy 2019-05-28 14:08 ` Roman Khabibov 2019-06-02 17:09 ` Vladislav Shpilevoy 2019-06-04 14:27 ` Roman Khabibov 2019-06-04 19:49 ` Vladislav Shpilevoy 2019-06-07 15:01 ` Roman Khabibov 2019-06-09 16:55 ` Vladislav Shpilevoy
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox