Tarantool development patches archive
 help / color / mirror / Atom feed
From: Roman Khabibov <roman.habibov@tarantool.org>
To: tarantool-patches@freelists.org
Cc: v.shpilevoy@tarantool.org
Subject: [tarantool-patches] [PATCH 2/2] sql: allow <COLLATE> only for string-like args
Date: Wed,  8 May 2019 14:29:16 +0300	[thread overview]
Message-ID: <015351a65570062bdd3577b7222f047f7de7e89b.1557312975.git.roman.habibov@tarantool.org> (raw)
In-Reply-To: <cover.1557312975.git.roman.habibov@tarantool.org>

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)

  parent reply	other threads:[~2019-05-08 11:29 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-08 11:29 [tarantool-patches] [PATCH 0/2] " 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 ` Roman Khabibov [this message]
2019-05-12 16:32   ` [tarantool-patches] Re: [PATCH 2/2] sql: allow <COLLATE> only for string-like args 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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=015351a65570062bdd3577b7222f047f7de7e89b.1557312975.git.roman.habibov@tarantool.org \
    --to=roman.habibov@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [tarantool-patches] [PATCH 2/2] sql: allow <COLLATE> only for string-like args' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox