[Tarantool-patches] [PATCH v1 2/2] sql: fix comparison in IN with subselect

[imeevma@tarantool.org]

After this patch, the IN statement checks the compatibility of the
values from subselect ​​on the right with the value on the left. If
values from subselect contains a value that is not comparable with
the left value, it throws an error.

Closes #4692
---
 src/box/sql/expr.c                                 | 27 ++++++++++++++++++++++
 .../gh-4692-comparison-in-IN-operator.test.lua     | 24 ++++++++++++++++++-
 test/sql-tap/in3.test.lua                          |  2 +-
 test/sql-tap/subquery.test.lua                     |  8 +++----
 test/sql-tap/tkt-80e031a00f.test.lua               |  8 +++----
 test/sql/boolean.result                            | 12 +++++-----
 6 files changed, 65 insertions(+), 16 deletions(-)

diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index 4fe4f83..c20c04b 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -2711,6 +2711,28 @@ expr_in_type(Parse *pParse, Expr *pExpr)
 	return zRet;
 }
 
+static inline bool
+is_in_type_cmp(struct Parse *parse, enum field_type lhs_type, struct Expr *expr)
+{
+	uint32_t fieldno = expr->iColumn;
+	enum field_type rhs_type = expr->space_def == NULL ?
+				   rhs_type = expr->type :
+				   expr->space_def->fields[fieldno].type;
+	if (rhs_type == lhs_type)
+		return true;
+	if (rhs_type == FIELD_TYPE_ANY || lhs_type == FIELD_TYPE_ANY)
+		return true;
+	if (rhs_type == FIELD_TYPE_SCALAR || lhs_type == FIELD_TYPE_SCALAR)
+		return true;
+	if (sql_type_is_numeric(rhs_type) && sql_type_is_numeric(lhs_type))
+		return true;
+	parse->is_aborted = true;
+	diag_set(ClientError, ER_SQL_TYPE_MISMATCH, field_type_strs[rhs_type],
+		 field_type_strs[lhs_type]);
+	return false;
+}
+
+
 /*
  * Generate code for scalar subqueries used as a subquery expression, EXISTS,
  * or IN operators.  Examples:
@@ -2821,6 +2843,11 @@ sqlCodeSubselect(Parse * pParse,	/* Parsing context */
 							      pExpr->iTable, reg_eph);
 					dest.dest_type =
 						expr_in_type(pParse, pExpr);
+					if (nVal == 1 &&
+					    !is_in_type_cmp(pParse,
+							    sql_expr_type(pLeft),
+							    pEList->a[0].pExpr))
+						return 0;
 					assert((pExpr->iTable & 0x0000FFFF) ==
 					       pExpr->iTable);
 					pSelect->iLimit = 0;
diff --git a/test/sql-tap/gh-4692-comparison-in-IN-operator.test.lua b/test/sql-tap/gh-4692-comparison-in-IN-operator.test.lua
index 14ba7ad..adab1e8 100755
--- a/test/sql-tap/gh-4692-comparison-in-IN-operator.test.lua
+++ b/test/sql-tap/gh-4692-comparison-in-IN-operator.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(2)
+test:plan(4)
 
 --
 -- If left value of IN and NOT IN operators is not a vector with
@@ -28,5 +28,27 @@ test:do_catchsql_test(
         -- </in4-1.1>
     })
 
+test:do_catchsql_test(
+    "gh-3692-2.1",
+    [[
+        CREATE TABLE t(i INT PRIMARY KEY, a BOOLEAN, b VARBINARY);
+        INSERT INTO t VALUES (1, true, X'31');
+        SELECT true in (SELECT b FROM t);
+    ]], {
+        -- <in4-1.1>
+        1, "Type mismatch: can not convert varbinary to boolean"
+        -- </in4-1.1>
+    })
+
+test:do_catchsql_test(
+    "gh-3692-2.2",
+    [[
+        SELECT X'31' in (SELECT a FROM t);
+    ]], {
+        -- <in4-1.1>
+        1, "Type mismatch: can not convert boolean to varbinary"
+        -- </in4-1.1>
+    })
+
 test:finish_test()
 
diff --git a/test/sql-tap/in3.test.lua b/test/sql-tap/in3.test.lua
index e29db9d..e4886f7 100755
--- a/test/sql-tap/in3.test.lua
+++ b/test/sql-tap/in3.test.lua
@@ -375,7 +375,7 @@ test:do_test(
     function()
         -- Numeric affinity is applied before the comparison takes place. 
         -- Making it impossible to use index t1_i3.
-        return exec_neph(" SELECT y IN (SELECT c FROM t1) FROM t2 ")
+        return exec_neph(" SELECT y IN (SELECT CAST(c AS INTEGER) FROM t1) FROM t2 ")
     end, {
         -- <in3-3.7>
         1, true
diff --git a/test/sql-tap/subquery.test.lua b/test/sql-tap/subquery.test.lua
index 6bedf58..e03a6a0 100755
--- a/test/sql-tap/subquery.test.lua
+++ b/test/sql-tap/subquery.test.lua
@@ -358,12 +358,12 @@ test:do_test(
         -- comparision. Hence, the integer value 10 in t3 will compare equal
         -- to the string value '10.0' in t4 because the t4 value will be
         -- converted into an integer.
-        return test:execsql [[
+        return test:catchsql [[
             SELECT * FROM t4 WHERE x IN (SELECT a FROM t3);
         ]]
     end, {
         -- <subquery-2.5.2>
-        "10.0"
+        1, "Type mismatch: can not convert integer to string"
         -- </subquery-2.5.2>
     })
 
@@ -372,13 +372,13 @@ test:do_test(
     function()
         -- The t4i index cannot be used to resolve the "x IN (...)" constraint
         -- because the constraint has integer affinity but t4i has text affinity.
-        return test:execsql [[
+        return test:catchsql [[
             CREATE INDEX t4i ON t4(x);
             SELECT * FROM t4 WHERE x IN (SELECT a FROM t3);
         ]]
     end, {
         -- <subquery-2.5.3.1>
-        "10.0"
+        1, "Type mismatch: can not convert integer to string"
         -- </subquery-2.5.3.1>
     })
 
diff --git a/test/sql-tap/tkt-80e031a00f.test.lua b/test/sql-tap/tkt-80e031a00f.test.lua
index a0e6539..2e0a921 100755
--- a/test/sql-tap/tkt-80e031a00f.test.lua
+++ b/test/sql-tap/tkt-80e031a00f.test.lua
@@ -346,7 +346,7 @@ test:do_catchsql_test(
         SELECT 'hello' IN t1
     ]], {
         -- <tkt-80e031a00f.27>
-        1, 'Type mismatch: can not convert hello to integer'
+        1, 'Type mismatch: can not convert integer to string'
         -- </tkt-80e031a00f.27>
     })
 
@@ -356,7 +356,7 @@ test:do_catchsql_test(
         SELECT 'hello' NOT IN t1
     ]], {
         -- <tkt-80e031a00f.28>
-        1, 'Type mismatch: can not convert hello to integer'
+        1, 'Type mismatch: can not convert integer to string'
         -- </tkt-80e031a00f.28>
     })
 
@@ -386,7 +386,7 @@ test:do_catchsql_test(
         SELECT x'303132' IN t1
     ]], {
         -- <tkt-80e031a00f.31>
-        1, 'Type mismatch: can not convert varbinary to integer'
+        1, 'Type mismatch: can not convert integer to varbinary'
         -- </tkt-80e031a00f.31>
     })
 
@@ -396,7 +396,7 @@ test:do_catchsql_test(
         SELECT x'303132' NOT IN t1
     ]], {
         -- <tkt-80e031a00f.32>
-        1, 'Type mismatch: can not convert varbinary to integer'
+        1, 'Type mismatch: can not convert integer to varbinary'
         -- </tkt-80e031a00f.32>
     })
 
diff --git a/test/sql/boolean.result b/test/sql/boolean.result
index b893f2a..112f0ac 100644
--- a/test/sql/boolean.result
+++ b/test/sql/boolean.result
@@ -3859,12 +3859,12 @@ SELECT false IN (0, 1, 2, 3);
 SELECT true IN (SELECT b FROM t7);
  | ---
  | - null
- | - 'Type mismatch: can not convert TRUE to integer'
+ | - 'Type mismatch: can not convert integer to boolean'
  | ...
 SELECT false IN (SELECT b FROM t7);
  | ---
  | - null
- | - 'Type mismatch: can not convert FALSE to integer'
+ | - 'Type mismatch: can not convert integer to boolean'
  | ...
 SELECT a1, a1 IN (0, 1, 2, 3) FROM t6
  | ---
@@ -5002,22 +5002,22 @@ SELECT a2 IN (0.1, 1.2, 2.3, 3.4) FROM t6 LIMIT 1;
 SELECT true IN (SELECT c FROM t8);
  | ---
  | - null
- | - 'Type mismatch: can not convert TRUE to number'
+ | - 'Type mismatch: can not convert number to boolean'
  | ...
 SELECT false IN (SELECT c FROM t8);
  | ---
  | - null
- | - 'Type mismatch: can not convert FALSE to number'
+ | - 'Type mismatch: can not convert number to boolean'
  | ...
 SELECT a1 IN (SELECT c FROM t8) FROM t6 LIMIT 1;
  | ---
  | - null
- | - 'Type mismatch: can not convert FALSE to number'
+ | - 'Type mismatch: can not convert number to boolean'
  | ...
 SELECT a2 IN (SELECT c FROM t8) FROM t6 LIMIT 1;
  | ---
  | - null
- | - 'Type mismatch: can not convert TRUE to number'
+ | - 'Type mismatch: can not convert number to boolean'
  | ...
 
 SELECT true BETWEEN 0.1 and 9.9;
-- 
2.7.4