From 86e80544436407387e7503cc1d1c8811675c5d38 Mon Sep 17 00:00:00 2001
From: Ivan Koptelov <ivan.koptelov@tarantool.org>
Date: Mon, 18 Mar 2019 14:49:33 +0300
Subject: [PATCH] sql: add better collation determination in functions
Before the patch determination of collation in SQL functions
was too narrow: the arguments were scanned
from left to right, till the argument with collation was
encountered, then its collation was used.
Now every arguments collation is considered. The right collation
which would be used in function is determined using ANSI
compatibility rules ("type combination" rules).
---
src/box/sql/expr.c | 46 +++++++++++--
src/box/sql/sqlInt.h | 8 ++-
test/sql-tap/func5.test.lua | 132 +++++++++++++++++++++++++++++++++++-
3 files changed, 177 insertions(+), 9 deletions(-)
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index 838fbd21ad..88c4a9f537 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -4081,16 +4081,48 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
testcase(i == 31);
constMask |= MASKBIT32(i);
}
- if ((pDef->funcFlags & SQL_FUNC_NEEDCOLL) !=
- 0 && coll == NULL) {
- bool unused;
- uint32_t id;
+ }
+ /*
+ * Function arguments may have different
+ * collations. The following code
+ * checks if they are compatible and
+ * finds the collation to be used. This
+ * is done using ANSI rules from
+ * collations_check_compatibility().
+ */
+ if ((pDef->funcFlags & SQL_FUNC_NEEDCOLL) != 0) {
+ struct coll *unused = NULL;
+ uint32_t curr_id = COLL_NONE;
+ bool is_curr_forced = false;
+
+ uint32_t next_id = COLL_NONE;
+ bool is_next_forced = false;
+
+ if (sql_expr_coll(pParse, pFarg->a[0].pExpr,
+ &is_curr_forced, &curr_id,
+ &unused) != 0)
+ return 0;
+
+ for (int j = 1; j < nFarg; j++) {
if (sql_expr_coll(pParse,
- pFarg->a[i].pExpr,
- &unused, &id,
- &coll) != 0)
+ pFarg->a[j].pExpr,
+ &is_next_forced,
+ &next_id,
+ &unused) != 0)
return 0;
+
+ if (collations_check_compatibility(
+ curr_id, is_curr_forced,
+ next_id, is_next_forced,
+ &curr_id) != 0) {
+ pParse->is_aborted = true;
+ return 0;
+ }
+ is_curr_forced = curr_id == next_id ?
+ is_next_forced :
+ is_curr_forced;
}
+ coll = coll_by_id(curr_id)->coll;
}
if (pFarg) {
if (constMask) {
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 72bd4ee0fe..2c07492c14 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -1661,7 +1661,13 @@ struct FuncDestructor {
#define SQL_FUNC_LIKE 0x0004 /* Candidate for the LIKE optimization */
#define SQL_FUNC_CASE 0x0008 /* Case-sensitive LIKE-type function */
#define SQL_FUNC_EPHEM 0x0010 /* Ephemeral. Delete with VDBE */
-#define SQL_FUNC_NEEDCOLL 0x0020 /* sqlGetFuncCollSeq() might be called */
+#define SQL_FUNC_NEEDCOLL 0x0020 /* sqlGetFuncCollSeq() might be called.
+ * The flag is set when the collation
+ * of function arguments should be
+ * determined, using rules in
+ * collations_check_compatibility()
+ * function.
+ */
#define SQL_FUNC_LENGTH 0x0040 /* Built-in length() function */
#define SQL_FUNC_TYPEOF 0x0080 /* Built-in typeof() function */
#define SQL_FUNC_COUNT 0x0100 /* Built-in count(*) aggregate */
diff --git a/test/sql-tap/func5.test.lua b/test/sql-tap/func5.test.lua
index bb9e11a049..337bd3b949 100755
--- a/test/sql-tap/func5.test.lua
+++ b/test/sql-tap/func5.test.lua
@@ -1,6 +1,6 @@
#!/usr/bin/env tarantool
test = require("sqltester")
-test:plan(5)
+test:plan(15)
--!./tcltestrunner.lua
-- 2010 August 27
@@ -98,5 +98,135 @@ test:do_execsql_test(
-- </func5-2.2>
})
+-- The following tests ensures that max() and min() functions
+-- raise error if argument's collations are incompatible.
+
+test:do_catchsql_test(
+ "func-5-3.1",
+ [[
+ SELECT max('a' COLLATE "unicode", 'A' COLLATE "unicode_ci");
+ ]],
+ {
+ -- <func5-3.1>
+ 1, "Illegal mix of collations"
+ -- </func5-3.1>
+ }
+)
+
+test:do_catchsql_test(
+ "func-5-3.2",
+ [[
+ CREATE TABLE test1 (s1 VARCHAR(5) PRIMARY KEY COLLATE "unicode");
+ CREATE TABLE test2 (s2 VARCHAR(5) PRIMARY KEY COLLATE "unicode_ci");
+ INSERT INTO test1 VALUES ('a');
+ INSERT INTO test2 VALUES ('a');
+ SELECT max(s1, s2) FROM test1 JOIN test2;
+ ]],
+ {
+ -- <func5-3.2>
+ 1, "Illegal mix of collations"
+ -- </func5-3.2>
+ }
+)
+
+test:do_catchsql_test(
+ "func-5-3.3",
+ [[
+ SELECT max ('abc', 'asd' COLLATE "binary", 'abc' COLLATE "unicode")
+ ]],
+ {
+ -- <func5-3.3>
+ 1, "Illegal mix of collations"
+ -- </func5-3.3>
+ }
+)
+
+test:do_execsql_test(
+ "func-5-3.4",
+ [[
+ SELECT max (s1, 'asd' COLLATE "binary", s2) FROM test1 JOIN test2;
+ ]], {
+ -- <func5-3.4>
+ "asd"
+ -- </func5-3.4>
+ }
+)
+
+test:do_catchsql_test(
+ "func-5.3.5",
+ [[
+ CREATE TABLE test3 (s3 VARCHAR(5) PRIMARY KEY COLLATE "unicode");
+ CREATE TABLE test4 (s4 VARCHAR(5) PRIMARY KEY COLLATE "unicode");
+ CREATE TABLE test5 (s5 VARCHAR(5) PRIMARY KEY COLLATE "binary");
+ INSERT INTO test3 VALUES ('a');
+ INSERT INTO test4 VALUES ('a');
+ INSERT INTO test5 VALUES ('a');
+ SELECT max(s3, s4, s5) FROM test3 JOIN test4 JOIN test5;
+ ]],
+ {
+ -- <func5-3.5>
+ 1, "Illegal mix of collations"
+ -- </func5-3.5>
+ }
+)
+
+test:do_catchsql_test(
+ "func-5-3.6",
+ [[
+ SELECT min('a' COLLATE "unicode", 'A' COLLATE "unicode_ci");
+ ]],
+ {
+ -- <func5-3.6>
+ 1, "Illegal mix of collations"
+ -- </func5-3.6>
+ }
+)
+
+test:do_catchsql_test(
+ "func-5-3.7",
+ [[
+ SELECT min(s1, s2) FROM test1 JOIN test2;
+ ]],
+ {
+ -- <func5-3.7>
+ 1, "Illegal mix of collations"
+ -- </func5-3.7>
+ }
+)
+
+test:do_catchsql_test(
+ "func-5-3.8",
+ [[
+ SELECT min ('abc', 'asd' COLLATE "binary", 'abc' COLLATE "unicode")
+ ]],
+ {
+ -- <func5-3.8>
+ 1, "Illegal mix of collations"
+ -- </func5-3.8>
+ }
+)
+
+test:do_execsql_test(
+ "func-5-3.9",
+ [[
+ SELECT min (s1, 'asd' COLLATE "binary", s2) FROM test1 JOIN test2;
+ ]], {
+ -- <func5-3.9>
+ "a"
+ -- </func5-3.9>
+ }
+)
+
+test:do_catchsql_test(
+ "func-5.3.10",
+ [[
+ SELECT min(s3, s4, s5) FROM test3 JOIN test4 JOIN test5;
+ ]],
+ {
+ -- <func5-3.10>
+ 1, "Illegal mix of collations"
+ -- <func5-3.10>
+ }
+)
test:finish_test()
--
GitLab