[Tarantool-patches] [PATCH v1 0/2] sql: fix comparison in IN operator

[Tarantool-patches] [PATCH v1 0/2] sql: fix comparison in IN operator

[imeevma]

Prior to this patch-set, comparisons in the IN statement were
performed using SCALAR rules in most cases. This patch-set changes
this behavior for the case when the left value is not a vector
whose length is greater than one. Now the left value is searched
among the right values ​​only if they are comparable. If there is a
value that is not comparable with the left value, the operation
throws an error.

https://github.com/tarantool/tarantool/issues/4692
https://github.com/tarantool/tarantool/tree/imeevma/gh-4692-specify-field-types-in-IN-statement

#ChangeLog
 - The IN operator now always throws an error if there is a value
   on the right that is not comparable with the left value (gh-4256).

Mergen Imeev (2):
  sql: fix comparison in IN with list of values
  sql: fix comparison in IN with subselect

 src/box/sql/expr.c                                 | 43 ++++++++++-
 .../gh-4692-comparison-in-IN-operator.test.lua     | 54 +++++++++++++
 test/sql-tap/in3.test.lua                          |  2 +-
 test/sql-tap/in4.test.lua                          |  4 +-
 test/sql-tap/subquery.test.lua                     |  8 +-
 test/sql-tap/tkt-80e031a00f.test.lua               |  8 +-
 test/sql/boolean.result                            | 88 +++++++---------------
 7 files changed, 132 insertions(+), 75 deletions(-)
 create mode 100755 test/sql-tap/gh-4692-comparison-in-IN-operator.test.lua

-- 
2.7.4

[Tarantool-patches] [PATCH v1 1/2] sql: fix comparison in IN with list of values

[imeevma]

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

Part of #4692
---
 src/box/sql/expr.c                                 | 16 ++++-
 .../gh-4692-comparison-in-IN-operator.test.lua     | 32 +++++++++
 test/sql-tap/in4.test.lua                          |  4 +-
 test/sql/boolean.result                            | 76 ++++++----------------
 4 files changed, 68 insertions(+), 60 deletions(-)
 create mode 100755 test/sql-tap/gh-4692-comparison-in-IN-operator.test.lua

diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index bc21824..4fe4f83 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -2847,6 +2847,7 @@ sqlCodeSubselect(Parse * pParse,	/* Parsing context */
 					}
 				}
 			} else if (ALWAYS(pExpr->x.pList != 0)) {
+				assert(nVal == 1);
 				/* Case 2:     expr IN (exprlist)
 				 *
 				 * For each expression, build an index key from the evaluation and
@@ -2861,6 +2862,9 @@ sqlCodeSubselect(Parse * pParse,	/* Parsing context */
 
 				enum field_type lhs_type =
 					sql_expr_type(pLeft);
+				enum field_type field_type =
+					sql_type_is_numeric(lhs_type) ?
+					FIELD_TYPE_NUMBER : lhs_type;
 				bool unused;
 				struct coll *unused_coll;
 				if (sql_expr_coll(pParse, pExpr->pLeft, &unused,
@@ -2886,8 +2890,16 @@ sqlCodeSubselect(Parse * pParse,	/* Parsing context */
 						jmpIfDynamic = -1;
 					}
 					r3 = sqlExprCodeTarget(pParse, pE2, r1);
-					enum field_type types[2] =
-						{ lhs_type, field_type_MAX };
+					size_t sz = 2 * sizeof(enum field_type);
+					enum field_type *types =
+						sqlDbMallocRaw(sql_get(), sz);
+					if (types == NULL)
+						return 0;
+					types[0] = field_type;
+					types[1] = field_type_MAX;
+					sqlVdbeAddOp4(v, OP_ApplyType, r3, 1, 0,
+						      (char *)types,
+						      P4_DYNAMIC);
 	 				sqlVdbeAddOp4(v, OP_MakeRecord, r3,
 							  1, r2, (char *)types,
 							  sizeof(types));
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
new file mode 100755
index 0000000..14ba7ad
--- /dev/null
+++ b/test/sql-tap/gh-4692-comparison-in-IN-operator.test.lua
@@ -0,0 +1,32 @@
+#!/usr/bin/env tarantool
+test = require("sqltester")
+test:plan(2)
+
+--
+-- If left value of IN and NOT IN operators is not a vector with
+-- length more that one, make sure that it cannot be compared to
+-- right values in case they are not comparable.
+--
+
+test:do_catchsql_test(
+    "gh-3692-1.1",
+    [[
+        SELECT true in (true, false, 1);
+    ]], {
+        -- <in4-1.1>
+        1, "Type mismatch: can not convert 1 to boolean"
+        -- </in4-1.1>
+    })
+
+test:do_catchsql_test(
+    "gh-3692-1.2",
+    [[
+        SELECT X'3132' in (X'31', X'32', 3);
+    ]], {
+        -- <in4-1.1>
+        1, "Type mismatch: can not convert 3 to varbinary"
+        -- </in4-1.1>
+    })
+
+test:finish_test()
+
diff --git a/test/sql-tap/in4.test.lua b/test/sql-tap/in4.test.lua
index 8c69173..2dd9b13 100755
--- a/test/sql-tap/in4.test.lua
+++ b/test/sql-tap/in4.test.lua
@@ -147,13 +147,13 @@ test:do_execsql_test(
         -- </in4-2.7>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "in4-2.8",
     [[
         SELECT b FROM t2 WHERE a IN ('', '0.0.0', '2') 
     ]], {
         -- <in4-2.8>
-        "two"
+        1, "Type mismatch: can not convert  to number"
         -- </in4-2.8>
     })
 
diff --git a/test/sql/boolean.result b/test/sql/boolean.result
index 112e41a..b893f2a 100644
--- a/test/sql/boolean.result
+++ b/test/sql/boolean.result
@@ -2256,19 +2256,13 @@ SELECT false IN (SELECT a1 FROM t6 LIMIT 1);
  | ...
 SELECT true IN (1, 1.2, 'true', false);
  | ---
- | - metadata:
- |   - name: true IN (1, 1.2, 'true', false)
- |     type: boolean
- |   rows:
- |   - [false]
+ | - null
+ | - 'Type mismatch: can not convert 1 to boolean'
  | ...
 SELECT false IN (1, 1.2, 'true', false);
  | ---
- | - metadata:
- |   - name: false IN (1, 1.2, 'true', false)
- |     type: boolean
- |   rows:
- |   - [true]
+ | - null
+ | - 'Type mismatch: can not convert 1 to boolean'
  | ...
 
 SELECT a, a IN (true) FROM t;
@@ -2328,14 +2322,8 @@ SELECT a, a IN (SELECT a1 FROM t6) FROM t;
  | ...
 SELECT a, a IN (1, 1.2, 'true', false) FROM t;
  | ---
- | - metadata:
- |   - name: A
- |     type: boolean
- |   - name: a IN (1, 1.2, 'true', false)
- |     type: boolean
- |   rows:
- |   - [false, true]
- |   - [true, false]
+ | - null
+ | - 'Type mismatch: can not convert 1 to boolean'
  | ...
 
 SELECT true BETWEEN true AND true;
@@ -3860,19 +3848,13 @@ SELECT a2, b, b != a2 FROM t6, t7;
 
 SELECT true IN (0, 1, 2, 3);
  | ---
- | - metadata:
- |   - name: true IN (0, 1, 2, 3)
- |     type: boolean
- |   rows:
- |   - [false]
+ | - null
+ | - 'Type mismatch: can not convert 0 to boolean'
  | ...
 SELECT false IN (0, 1, 2, 3);
  | ---
- | - metadata:
- |   - name: false IN (0, 1, 2, 3)
- |     type: boolean
- |   rows:
- |   - [false]
+ | - null
+ | - 'Type mismatch: can not convert 0 to boolean'
  | ...
 SELECT true IN (SELECT b FROM t7);
  | ---
@@ -3886,14 +3868,8 @@ SELECT false IN (SELECT b FROM t7);
  | ...
 SELECT a1, a1 IN (0, 1, 2, 3) FROM t6
  | ---
- | - metadata:
- |   - name: A1
- |     type: boolean
- |   - name: a1 IN (0, 1, 2, 3)
- |     type: boolean
- |   rows:
- |   - [false, false]
- |   - [true, false]
+ | - null
+ | - 'Type mismatch: can not convert 0 to boolean'
  | ...
 
 SELECT true BETWEEN 0 and 10;
@@ -5005,35 +4981,23 @@ SELECT a2, c, c != a2 FROM t6, t8;
 
 SELECT true IN (0.1, 1.2, 2.3, 3.4);
  | ---
- | - metadata:
- |   - name: true IN (0.1, 1.2, 2.3, 3.4)
- |     type: boolean
- |   rows:
- |   - [false]
+ | - null
+ | - 'Type mismatch: can not convert 0.1 to boolean'
  | ...
 SELECT false IN (0.1, 1.2, 2.3, 3.4);
  | ---
- | - metadata:
- |   - name: false IN (0.1, 1.2, 2.3, 3.4)
- |     type: boolean
- |   rows:
- |   - [false]
+ | - null
+ | - 'Type mismatch: can not convert 0.1 to boolean'
  | ...
 SELECT a1 IN (0.1, 1.2, 2.3, 3.4) FROM t6 LIMIT 1;
  | ---
- | - metadata:
- |   - name: a1 IN (0.1, 1.2, 2.3, 3.4)
- |     type: boolean
- |   rows:
- |   - [false]
+ | - null
+ | - 'Type mismatch: can not convert 0.1 to boolean'
  | ...
 SELECT a2 IN (0.1, 1.2, 2.3, 3.4) FROM t6 LIMIT 1;
  | ---
- | - metadata:
- |   - name: a2 IN (0.1, 1.2, 2.3, 3.4)
- |     type: boolean
- |   rows:
- |   - [false]
+ | - null
+ | - 'Type mismatch: can not convert 0.1 to boolean'
  | ...
 SELECT true IN (SELECT c FROM t8);
  | ---
-- 
2.7.4

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

[imeevma]

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

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

[Vladislav Shpilevoy]

Hi! Thanks for the patch!

See 6 comments below.

On 15/04/2020 13:47, imeevma@tarantool.org wrote:
> After this patch, the IN statement checks the compatibility of the
> values from subselect ​​on the right with the value on the left. If

1. I see broken unicode after 'subselect' on this line, when I do
'git log'.

> 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)

2. Sorry, can't parse the name. Can't understand what is 'is cmp'.
Could you please rename and add a comment maybe?

> +{
> +	uint32_t fieldno = expr->iColumn;
> +	enum field_type rhs_type = expr->space_def == NULL ?
> +				   rhs_type = expr->type :

3. You basically wrote 'rhs_type = (rhs_type = expr->type)'. You
may need to remove the second 'rhs_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]);

4. Wouldn't it be the same to check

    field_type1_contains_type2(lhs_type, rhs_type) ||
    field_type1_contains_type2(rhs_type, 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 &&

5. What if it is not 1? How types are checked then?

> +					    !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/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 [[

6. The comment above is outdated. Probably better to remove this test,
since it no longer serves its purpose. Or change the comment. Please,
check other changed tests too. Maybe they also became irrelevant or their
comments are misleading.

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

[Mergen Imeev]

Hi! Thank you for review. My answers, diff and new patch
below.

On Sun, Apr 19, 2020 at 07:47:23PM +0200, Vladislav Shpilevoy wrote:
> Hi! Thanks for the patch!
> 
> See 6 comments below.
> 
> On 15/04/2020 13:47, imeevma@tarantool.org wrote:
> > After this patch, the IN statement checks the compatibility of the
> > values from subselect ​​on the right with the value on the left. If
> 
> 1. I see broken unicode after 'subselect' on this line, when I do
> 'git log'.
> 
Fixed.

> > 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)
> 
> 2. Sorry, can't parse the name. Can't understand what is 'is cmp'.
> Could you please rename and add a comment maybe?
> 
Fixed. I gave it a new name, 'is_comparable'. It is not a
lot better, still since it is just a static inline function,
will this be enough?

> > +{
> > +	uint32_t fieldno = expr->iColumn;
> > +	enum field_type rhs_type = expr->space_def == NULL ?
> > +				   rhs_type = expr->type :
> 
> 3. You basically wrote 'rhs_type = (rhs_type = expr->type)'. You
> may need to remove the second 'rhs_type = '.
> 
Sorry, fixed.

> > +				   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]);
> 
> 4. Wouldn't it be the same to check
> 
>     field_type1_contains_type2(lhs_type, rhs_type) ||
>     field_type1_contains_type2(rhs_type, lhs_type)
> 
> ?
> 
Not exactly, since we can compare INTEGERs with DOUBLEs.
However, thank you for this suggestion!

> > +	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 &&
> 
> 5. What if it is not 1? How types are checked then?
> 
Fixed. Nikita suggested to do this only to values whose
length is 1 since it should be simpler. Still, it appears
to be not harder to check all fields. Also, I found out
that my previous solution didn't worked as intended in case
there is a vector or a SELECT on the left of IN.

> > +					    !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/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 [[
> 
> 6. The comment above is outdated. Probably better to remove this test,
> since it no longer serves its purpose. Or change the comment. Please,
> check other changed tests too. Maybe they also became irrelevant or their
> comments are misleading.
I removed comments for all test I changed since they all
were outdated.


Diff:

commit 3c01abf7d969dfaf72b3798269032f32b6d74356
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Wed Apr 22 10:52:46 2020 +0300

    Review fix

diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index c20c04b..9cd3a2c 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -2711,28 +2711,49 @@ expr_in_type(Parse *pParse, Expr *pExpr)
 	return zRet;
 }
 
+/**
+ * Check that arguments on both sides of IN are comparable.
+ *
+ * @param parse Parsing context.
+ * @param pExpr Expr that contains all operands.
+ *
+ * @retval true if comparable, false otherwise.
+ */
 static inline bool
-is_in_type_cmp(struct Parse *parse, enum field_type lhs_type, struct Expr *expr)
+is_comparable(struct Parse *parse, struct Expr *pExpr)
 {
-	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;
+	uint32_t size = sqlExprVectorSize(pExpr->pLeft);
+	ExprList *lhs_list = NULL;
+	ExprList *rhs_list = pExpr->x.pSelect->pEList;
+	u8 op = pExpr->pLeft->op;
+	if (op == TK_REGISTER)
+		op = pExpr->pLeft->op2;
+	if (op == TK_VECTOR)
+		lhs_list = pExpr->pLeft->x.pList;
+	else if (op == TK_SELECT)
+		lhs_list = pExpr->pLeft->x.pSelect->pEList;
+	assert(size == 1 || lhs_list != NULL);
+
+	for (uint32_t i = 0; i < size; ++i) {
+		struct Expr *lhs = lhs_list == NULL ? pExpr->pLeft :
+				   lhs_list->a[i].pExpr;
+		struct Expr *rhs = rhs_list->a[i].pExpr;
+		enum field_type lhs_type = sql_expr_type(lhs);
+		enum field_type rhs_type = sql_expr_type(rhs);
+		if (field_type1_contains_type2(lhs_type, rhs_type) ||
+		    field_type1_contains_type2(rhs_type, lhs_type))
+			continue;
+		if (sql_type_is_numeric(rhs_type) &&
+		    sql_type_is_numeric(lhs_type))
+			continue;
+		parse->is_aborted = true;
+		diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
+			 field_type_strs[rhs_type], field_type_strs[lhs_type]);
+		return false;
+	}
+	return true;
 }
 
-
 /*
  * Generate code for scalar subqueries used as a subquery expression, EXISTS,
  * or IN operators.  Examples:
@@ -2843,10 +2864,7 @@ 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))
+					if (!is_comparable(pParse, pExpr))
 						return 0;
 					assert((pExpr->iTable & 0x0000FFFF) ==
 					       pExpr->iTable);
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 ef76290..a03b688 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(4)
+test:plan(11)
 
 --
 -- If left value of IN and NOT IN operators is not a vector with
@@ -25,25 +25,80 @@ test:do_catchsql_test(
     })
 
 test:do_catchsql_test(
-    "gh-3692-2.1",
+    "gh-4692-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);
+        SELECT true IN (SELECT X'31');
     ]], {
-        -- <in4-1.1>
         1, "Type mismatch: can not convert varbinary to boolean"
-        -- </in4-1.1>
     })
 
 test:do_catchsql_test(
-    "gh-3692-2.2",
+    "gh-4692-2.2",
     [[
-        SELECT X'31' in (SELECT a FROM t);
+        SELECT X'31' IN (SELECT true);
     ]], {
-        -- <in4-1.1>
         1, "Type mismatch: can not convert boolean to varbinary"
-        -- </in4-1.1>
+    })
+
+test:do_catchsql_test(
+    "gh-4692-2.3",
+    [[
+        SELECT (1, 'a') IN (SELECT 1, 2);
+    ]], {
+        1, "Type mismatch: can not convert integer to string"
+    })
+
+test:do_catchsql_test(
+    "gh-4692-2.4",
+    [[
+        SELECT (SELECT 1, 'a') IN (SELECT 1, 2);
+    ]], {
+        1, "Type mismatch: can not convert integer to string"
+    })
+
+test:execsql([[
+        CREATE TABLE t (i INT PRIMARY KEY, a BOOLEAN, b VARBINARY);
+        INSERT INTO t VALUES(1, true, X'31');
+    ]])
+
+test:do_catchsql_test(
+    "gh-4692-2.5",
+    [[
+        SELECT true IN (SELECT b FROM t);
+    ]], {
+        1, "Type mismatch: can not convert varbinary to boolean"
+    })
+
+test:do_catchsql_test(
+    "gh-4692-2.6",
+    [[
+        SELECT X'31' IN (SELECT a FROM t);
+    ]], {
+        1, "Type mismatch: can not convert boolean to varbinary"
+    })
+
+test:do_catchsql_test(
+    "gh-4692-2.7",
+    [[
+        SELECT (1, 'a') IN (SELECT i, a from t);
+    ]], {
+        1, "Type mismatch: can not convert boolean to string"
+    })
+
+test:do_catchsql_test(
+    "gh-4692-2.8",
+    [[
+        SELECT (SELECT 1, 'a') IN (SELECT i, a from t);
+    ]], {
+        1, "Type mismatch: can not convert boolean to string"
+    })
+
+test:do_catchsql_test(
+    "gh-4692-2.9",
+    [[
+        SELECT (SELECT i, a from t) IN (SELECT a, i from t);
+    ]], {
+        1, "Type mismatch: can not convert boolean to integer"
     })
 
 test:finish_test()
diff --git a/test/sql-tap/in3.test.lua b/test/sql-tap/in3.test.lua
index e4886f7..e3f8682 100755
--- a/test/sql-tap/in3.test.lua
+++ b/test/sql-tap/in3.test.lua
@@ -373,8 +373,6 @@ test:do_test(
 test:do_test(
     "in3-3.7",
     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 CAST(c AS INTEGER) FROM t1) FROM t2 ")
     end, {
         -- <in3-3.7>
diff --git a/test/sql-tap/subquery.test.lua b/test/sql-tap/subquery.test.lua
index a5a8e95..527ea2b 100755
--- a/test/sql-tap/subquery.test.lua
+++ b/test/sql-tap/subquery.test.lua
@@ -352,12 +352,6 @@ test:do_execsql_test(
 test:do_test(
     "subquery-2.5.2",
     function()
-        -- In the expr "x IN (SELECT a FROM t3)" the RHS of the IN operator
-        -- has text affinity and the LHS has integer affinity.  The rule is
-        -- that we try to convert both sides to an integer before doing the
-        -- 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:catchsql [[
             SELECT * FROM t4 WHERE x IN (SELECT a FROM t3);
         ]]
@@ -370,8 +364,6 @@ test:do_test(
 test:do_test(
     "subquery-2.5.3.1",
     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:catchsql [[
             CREATE INDEX t4i ON t4(x);
             SELECT * FROM t4 WHERE x IN (SELECT a FROM t3);



New patch:

From 946cd381dd0af8ba1b534e9721417423f315a542 Mon Sep 17 00:00:00 2001
From: Mergen Imeev <imeevma@gmail.com>
Date: Wed, 15 Apr 2020 02:17:43 +0300
Subject: [PATCH] sql: fix comparison in IN with subselect

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

diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index 4fe4f83..9cd3a2c 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -2711,6 +2711,49 @@ expr_in_type(Parse *pParse, Expr *pExpr)
 	return zRet;
 }
 
+/**
+ * Check that arguments on both sides of IN are comparable.
+ *
+ * @param parse Parsing context.
+ * @param pExpr Expr that contains all operands.
+ *
+ * @retval true if comparable, false otherwise.
+ */
+static inline bool
+is_comparable(struct Parse *parse, struct Expr *pExpr)
+{
+	uint32_t size = sqlExprVectorSize(pExpr->pLeft);
+	ExprList *lhs_list = NULL;
+	ExprList *rhs_list = pExpr->x.pSelect->pEList;
+	u8 op = pExpr->pLeft->op;
+	if (op == TK_REGISTER)
+		op = pExpr->pLeft->op2;
+	if (op == TK_VECTOR)
+		lhs_list = pExpr->pLeft->x.pList;
+	else if (op == TK_SELECT)
+		lhs_list = pExpr->pLeft->x.pSelect->pEList;
+	assert(size == 1 || lhs_list != NULL);
+
+	for (uint32_t i = 0; i < size; ++i) {
+		struct Expr *lhs = lhs_list == NULL ? pExpr->pLeft :
+				   lhs_list->a[i].pExpr;
+		struct Expr *rhs = rhs_list->a[i].pExpr;
+		enum field_type lhs_type = sql_expr_type(lhs);
+		enum field_type rhs_type = sql_expr_type(rhs);
+		if (field_type1_contains_type2(lhs_type, rhs_type) ||
+		    field_type1_contains_type2(rhs_type, lhs_type))
+			continue;
+		if (sql_type_is_numeric(rhs_type) &&
+		    sql_type_is_numeric(lhs_type))
+			continue;
+		parse->is_aborted = true;
+		diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
+			 field_type_strs[rhs_type], field_type_strs[lhs_type]);
+		return false;
+	}
+	return true;
+}
+
 /*
  * Generate code for scalar subqueries used as a subquery expression, EXISTS,
  * or IN operators.  Examples:
@@ -2821,6 +2864,8 @@ sqlCodeSubselect(Parse * pParse,	/* Parsing context */
 							      pExpr->iTable, reg_eph);
 					dest.dest_type =
 						expr_in_type(pParse, pExpr);
+					if (!is_comparable(pParse, 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 b1d8a5f..a03b688 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(11)
 
 --
 -- If left value of IN and NOT IN operators is not a vector with
@@ -24,5 +24,82 @@ test:do_catchsql_test(
         1, "Type mismatch: can not convert 3 to varbinary"
     })
 
+test:do_catchsql_test(
+    "gh-4692-2.1",
+    [[
+        SELECT true IN (SELECT X'31');
+    ]], {
+        1, "Type mismatch: can not convert varbinary to boolean"
+    })
+
+test:do_catchsql_test(
+    "gh-4692-2.2",
+    [[
+        SELECT X'31' IN (SELECT true);
+    ]], {
+        1, "Type mismatch: can not convert boolean to varbinary"
+    })
+
+test:do_catchsql_test(
+    "gh-4692-2.3",
+    [[
+        SELECT (1, 'a') IN (SELECT 1, 2);
+    ]], {
+        1, "Type mismatch: can not convert integer to string"
+    })
+
+test:do_catchsql_test(
+    "gh-4692-2.4",
+    [[
+        SELECT (SELECT 1, 'a') IN (SELECT 1, 2);
+    ]], {
+        1, "Type mismatch: can not convert integer to string"
+    })
+
+test:execsql([[
+        CREATE TABLE t (i INT PRIMARY KEY, a BOOLEAN, b VARBINARY);
+        INSERT INTO t VALUES(1, true, X'31');
+    ]])
+
+test:do_catchsql_test(
+    "gh-4692-2.5",
+    [[
+        SELECT true IN (SELECT b FROM t);
+    ]], {
+        1, "Type mismatch: can not convert varbinary to boolean"
+    })
+
+test:do_catchsql_test(
+    "gh-4692-2.6",
+    [[
+        SELECT X'31' IN (SELECT a FROM t);
+    ]], {
+        1, "Type mismatch: can not convert boolean to varbinary"
+    })
+
+test:do_catchsql_test(
+    "gh-4692-2.7",
+    [[
+        SELECT (1, 'a') IN (SELECT i, a from t);
+    ]], {
+        1, "Type mismatch: can not convert boolean to string"
+    })
+
+test:do_catchsql_test(
+    "gh-4692-2.8",
+    [[
+        SELECT (SELECT 1, 'a') IN (SELECT i, a from t);
+    ]], {
+        1, "Type mismatch: can not convert boolean to string"
+    })
+
+test:do_catchsql_test(
+    "gh-4692-2.9",
+    [[
+        SELECT (SELECT i, a from t) IN (SELECT a, i from t);
+    ]], {
+        1, "Type mismatch: can not convert boolean to integer"
+    })
+
 test:finish_test()
 
diff --git a/test/sql-tap/in3.test.lua b/test/sql-tap/in3.test.lua
index e29db9d..e3f8682 100755
--- a/test/sql-tap/in3.test.lua
+++ b/test/sql-tap/in3.test.lua
@@ -373,9 +373,7 @@ test:do_test(
 test:do_test(
     "in3-3.7",
     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 15c4c82..527ea2b 100755
--- a/test/sql-tap/subquery.test.lua
+++ b/test/sql-tap/subquery.test.lua
@@ -352,33 +352,25 @@ test:do_execsql_test(
 test:do_test(
     "subquery-2.5.2",
     function()
-        -- In the expr "x IN (SELECT a FROM t3)" the RHS of the IN operator
-        -- has text affinity and the LHS has integer affinity.  The rule is
-        -- that we try to convert both sides to an integer before doing the
-        -- 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"
+        1, "Type mismatch: can not convert integer to string"
         -- </subquery-2.5.2>
     })
 
 test:do_test(
     "subquery-2.5.3.1",
     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"
+        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;

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

[Vladislav Shpilevoy]

Hi! Thanks for the fixes!

>>> 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)
>>
>> 2. Sorry, can't parse the name. Can't understand what is 'is cmp'.
>> Could you please rename and add a comment maybe?
>>
> Fixed. I gave it a new name, 'is_comparable'. It is not a
> lot better, still since it is just a static inline function,
> will this be enough?

I am afraid it is too common for a huge file like expr.c. Just
extend the old name "is_in_type_cmp()" -> "are_in_args_comparable()".

> From 946cd381dd0af8ba1b534e9721417423f315a542 Mon Sep 17 00:00:00 2001
> From: Mergen Imeev <imeevma@gmail.com>
> Date: Wed, 15 Apr 2020 02:17:43 +0300
> Subject: [PATCH] sql: fix comparison in IN with subselect
> 
> 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
> 
> diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
> index 4fe4f83..9cd3a2c 100644
> --- a/src/box/sql/expr.c
> +++ b/src/box/sql/expr.c
> @@ -2711,6 +2711,49 @@ expr_in_type(Parse *pParse, Expr *pExpr)
>  	return zRet;
>  }
>  
> +/**
> + * Check that arguments on both sides of IN are comparable.
> + *
> + * @param parse Parsing context.
> + * @param pExpr Expr that contains all operands.
> + *
> + * @retval true if comparable, false otherwise.
> + */
> +static inline bool
> +is_comparable(struct Parse *parse, struct Expr *pExpr)
> +{
> +	uint32_t size = sqlExprVectorSize(pExpr->pLeft);
> +	ExprList *lhs_list = NULL;
> +	ExprList *rhs_list = pExpr->x.pSelect->pEList;
> +	u8 op = pExpr->pLeft->op;
> +	if (op == TK_REGISTER)
> +		op = pExpr->pLeft->op2;
> +	if (op == TK_VECTOR)
> +		lhs_list = pExpr->pLeft->x.pList;
> +	else if (op == TK_SELECT)
> +		lhs_list = pExpr->pLeft->x.pSelect->pEList;
> +	assert(size == 1 || lhs_list != NULL);

What is happening on the lines above?

Also, I see similar code below is_comparable() invocation:

	for (i = 0; i < nVal; i++) {
		Expr *p =
		    sqlVectorFieldSubexpr
		    (pLeft, i);
		if (sql_binary_compare_coll_seq(pParse, p, pEList->a[i].pExpr,
						&key_info->parts[i].coll_id) != 0)
			return 0;
	}

It is ugly, but the point is they use sqlVectorFieldSubexpr(pLeft, i)
without any 'TK_VECTOR', 'TK_SELECT', 'TK_REGISTER' checks. How
does it work? And shouldn't we move this cycle into is_comparable(),
because seems like it also checks if types are comparable when they
are strings. However I am not sure.

> +
> +	for (uint32_t i = 0; i < size; ++i) {
> +		struct Expr *lhs = lhs_list == NULL ? pExpr->pLeft :
> +				   lhs_list->a[i].pExpr;
> +		struct Expr *rhs = rhs_list->a[i].pExpr;
> +		enum field_type lhs_type = sql_expr_type(lhs);
> +		enum field_type rhs_type = sql_expr_type(rhs);
> +		if (field_type1_contains_type2(lhs_type, rhs_type) ||
> +		    field_type1_contains_type2(rhs_type, lhs_type))
> +			continue;
> +		if (sql_type_is_numeric(rhs_type) &&
> +		    sql_type_is_numeric(lhs_type))
> +			continue;
> +		parse->is_aborted = true;
> +		diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
> +			 field_type_strs[rhs_type], field_type_strs[lhs_type]);
> +		return false;
> +	}
> +	return true;
> +}

Re: [Tarantool-patches] [PATCH v1 1/2] sql: fix comparison in IN with list of values

[Vladislav Shpilevoy]

> 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
> new file mode 100755
> index 0000000..14ba7ad
> --- /dev/null
> +++ b/test/sql-tap/gh-4692-comparison-in-IN-operator.test.lua
> @@ -0,0 +1,32 @@
> +#!/usr/bin/env tarantool
> +test = require("sqltester")
> +test:plan(2)
> +
> +--
> +-- If left value of IN and NOT IN operators is not a vector with
> +-- length more that one, make sure that it cannot be compared to

that -> than.

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

[Mergen Imeev]

Hi! Thank you for the review! My asnwers and new patch
below.

On Tue, Apr 28, 2020 at 12:53:11AM +0200, Vladislav Shpilevoy wrote:
> Hi! Thanks for the fixes!
> 
> >>> 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)
> >>
> >> 2. Sorry, can't parse the name. Can't understand what is 'is cmp'.
> >> Could you please rename and add a comment maybe?
> >>
> > Fixed. I gave it a new name, 'is_comparable'. It is not a
> > lot better, still since it is just a static inline function,
> > will this be enough?
> 
> I am afraid it is too common for a huge file like expr.c. Just
> extend the old name "is_in_type_cmp()" -> "are_in_args_comparable()".
> 
Thanks, fixed. I changed name to
are_in_args_types_comparable(). Also, expanded description
of the function. I think it is better to let this function
to check comparability of the types alone.

> > From 946cd381dd0af8ba1b534e9721417423f315a542 Mon Sep 17 00:00:00 2001
> > From: Mergen Imeev <imeevma@gmail.com>
> > Date: Wed, 15 Apr 2020 02:17:43 +0300
> > Subject: [PATCH] sql: fix comparison in IN with subselect
> > 
> > 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
> > 
> > diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
> > index 4fe4f83..9cd3a2c 100644
> > --- a/src/box/sql/expr.c
> > +++ b/src/box/sql/expr.c
> > @@ -2711,6 +2711,49 @@ expr_in_type(Parse *pParse, Expr *pExpr)
> >  	return zRet;
> >  }
> >  
> > +/**
> > + * Check that arguments on both sides of IN are comparable.
> > + *
> > + * @param parse Parsing context.
> > + * @param pExpr Expr that contains all operands.
> > + *
> > + * @retval true if comparable, false otherwise.
> > + */
> > +static inline bool
> > +is_comparable(struct Parse *parse, struct Expr *pExpr)
> > +{
> > +	uint32_t size = sqlExprVectorSize(pExpr->pLeft);
> > +	ExprList *lhs_list = NULL;
> > +	ExprList *rhs_list = pExpr->x.pSelect->pEList;
> > +	u8 op = pExpr->pLeft->op;
> > +	if (op == TK_REGISTER)
> > +		op = pExpr->pLeft->op2;
> > +	if (op == TK_VECTOR)
> > +		lhs_list = pExpr->pLeft->x.pList;
> > +	else if (op == TK_SELECT)
> > +		lhs_list = pExpr->pLeft->x.pSelect->pEList;
> > +	assert(size == 1 || lhs_list != NULL);
> 
> What is happening on the lines above?
> 
Before working with the left value, we need to find out if
it is a scalar, vector, or subselect. In the case of a
vector or a subselect, we should get a list of expressions
containing information about each field in the left value.
We will use this list to check comparability.

> Also, I see similar code below is_comparable() invocation:
> 
> 	for (i = 0; i < nVal; i++) {
> 		Expr *p =
> 		    sqlVectorFieldSubexpr
> 		    (pLeft, i);
> 		if (sql_binary_compare_coll_seq(pParse, p, pEList->a[i].pExpr,
> 						&key_info->parts[i].coll_id) != 0)
> 			return 0;
> 	}
> 
> It is ugly, but the point is they use sqlVectorFieldSubexpr(pLeft, i)
> without any 'TK_VECTOR', 'TK_SELECT', 'TK_REGISTER' checks. How
> does it work?
We do not need to check here since we already know that
the right side is a subselect.

> And shouldn't we move this cycle into is_comparable(),
> because seems like it also checks if types are comparable when they
> are strings. However I am not sure.
> 
It is true that compability of collations checked here, but
it returns ID of collation that should be selected after
this check. This id is used during creation of format and
index of ephemeral space.

> > +
> > +	for (uint32_t i = 0; i < size; ++i) {
> > +		struct Expr *lhs = lhs_list == NULL ? pExpr->pLeft :
> > +				   lhs_list->a[i].pExpr;
> > +		struct Expr *rhs = rhs_list->a[i].pExpr;
> > +		enum field_type lhs_type = sql_expr_type(lhs);
> > +		enum field_type rhs_type = sql_expr_type(rhs);
> > +		if (field_type1_contains_type2(lhs_type, rhs_type) ||
> > +		    field_type1_contains_type2(rhs_type, lhs_type))
> > +			continue;
> > +		if (sql_type_is_numeric(rhs_type) &&
> > +		    sql_type_is_numeric(lhs_type))
> > +			continue;
> > +		parse->is_aborted = true;
> > +		diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
> > +			 field_type_strs[rhs_type], field_type_strs[lhs_type]);
> > +		return false;
> > +	}
> > +	return true;
> > +}


New patch:


From 495d3c0b9fdb17ca99a4cae7528fa448d4ea07ac Mon Sep 17 00:00:00 2001
From: Mergen Imeev <imeevma@gmail.com>
Date: Wed, 15 Apr 2020 02:17:43 +0300
Subject: [PATCH] sql: fix comparison in IN with subselect

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

diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index 4fe4f83..59d128f 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -2711,6 +2711,51 @@ expr_in_type(Parse *pParse, Expr *pExpr)
 	return zRet;
 }
 
+/**
+ * Check that field types in arguments on both sides of IN are
+ * comparable. Note that it does not check that fields of type
+ * string have comparable collations.
+ *
+ * @param parse Parsing context.
+ * @param pExpr Expr that contains all operands.
+ *
+ * @retval true if comparable, false otherwise.
+ */
+static inline bool
+are_in_args_types_comparable(struct Parse *parse, struct Expr *pExpr)
+{
+	uint32_t size = sqlExprVectorSize(pExpr->pLeft);
+	ExprList *lhs_list = NULL;
+	ExprList *rhs_list = pExpr->x.pSelect->pEList;
+	u8 op = pExpr->pLeft->op;
+	if (op == TK_REGISTER)
+		op = pExpr->pLeft->op2;
+	if (op == TK_VECTOR)
+		lhs_list = pExpr->pLeft->x.pList;
+	else if (op == TK_SELECT)
+		lhs_list = pExpr->pLeft->x.pSelect->pEList;
+	assert(size == 1 || lhs_list != NULL);
+
+	for (uint32_t i = 0; i < size; ++i) {
+		struct Expr *lhs = lhs_list == NULL ? pExpr->pLeft :
+				   lhs_list->a[i].pExpr;
+		struct Expr *rhs = rhs_list->a[i].pExpr;
+		enum field_type lhs_type = sql_expr_type(lhs);
+		enum field_type rhs_type = sql_expr_type(rhs);
+		if (field_type1_contains_type2(lhs_type, rhs_type) ||
+		    field_type1_contains_type2(rhs_type, lhs_type))
+			continue;
+		if (sql_type_is_numeric(rhs_type) &&
+		    sql_type_is_numeric(lhs_type))
+			continue;
+		parse->is_aborted = true;
+		diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
+			 field_type_strs[rhs_type], field_type_strs[lhs_type]);
+		return false;
+	}
+	return true;
+}
+
 /*
  * Generate code for scalar subqueries used as a subquery expression, EXISTS,
  * or IN operators.  Examples:
@@ -2821,6 +2866,8 @@ sqlCodeSubselect(Parse * pParse,	/* Parsing context */
 							      pExpr->iTable, reg_eph);
 					dest.dest_type =
 						expr_in_type(pParse, pExpr);
+					if (!are_in_args_types_comparable(pParse, 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 b1d8a5f..a03b688 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(11)
 
 --
 -- If left value of IN and NOT IN operators is not a vector with
@@ -24,5 +24,82 @@ test:do_catchsql_test(
         1, "Type mismatch: can not convert 3 to varbinary"
     })
 
+test:do_catchsql_test(
+    "gh-4692-2.1",
+    [[
+        SELECT true IN (SELECT X'31');
+    ]], {
+        1, "Type mismatch: can not convert varbinary to boolean"
+    })
+
+test:do_catchsql_test(
+    "gh-4692-2.2",
+    [[
+        SELECT X'31' IN (SELECT true);
+    ]], {
+        1, "Type mismatch: can not convert boolean to varbinary"
+    })
+
+test:do_catchsql_test(
+    "gh-4692-2.3",
+    [[
+        SELECT (1, 'a') IN (SELECT 1, 2);
+    ]], {
+        1, "Type mismatch: can not convert integer to string"
+    })
+
+test:do_catchsql_test(
+    "gh-4692-2.4",
+    [[
+        SELECT (SELECT 1, 'a') IN (SELECT 1, 2);
+    ]], {
+        1, "Type mismatch: can not convert integer to string"
+    })
+
+test:execsql([[
+        CREATE TABLE t (i INT PRIMARY KEY, a BOOLEAN, b VARBINARY);
+        INSERT INTO t VALUES(1, true, X'31');
+    ]])
+
+test:do_catchsql_test(
+    "gh-4692-2.5",
+    [[
+        SELECT true IN (SELECT b FROM t);
+    ]], {
+        1, "Type mismatch: can not convert varbinary to boolean"
+    })
+
+test:do_catchsql_test(
+    "gh-4692-2.6",
+    [[
+        SELECT X'31' IN (SELECT a FROM t);
+    ]], {
+        1, "Type mismatch: can not convert boolean to varbinary"
+    })
+
+test:do_catchsql_test(
+    "gh-4692-2.7",
+    [[
+        SELECT (1, 'a') IN (SELECT i, a from t);
+    ]], {
+        1, "Type mismatch: can not convert boolean to string"
+    })
+
+test:do_catchsql_test(
+    "gh-4692-2.8",
+    [[
+        SELECT (SELECT 1, 'a') IN (SELECT i, a from t);
+    ]], {
+        1, "Type mismatch: can not convert boolean to string"
+    })
+
+test:do_catchsql_test(
+    "gh-4692-2.9",
+    [[
+        SELECT (SELECT i, a from t) IN (SELECT a, i from t);
+    ]], {
+        1, "Type mismatch: can not convert boolean to integer"
+    })
+
 test:finish_test()
 
diff --git a/test/sql-tap/in3.test.lua b/test/sql-tap/in3.test.lua
index e29db9d..e3f8682 100755
--- a/test/sql-tap/in3.test.lua
+++ b/test/sql-tap/in3.test.lua
@@ -373,9 +373,7 @@ test:do_test(
 test:do_test(
     "in3-3.7",
     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 15c4c82..527ea2b 100755
--- a/test/sql-tap/subquery.test.lua
+++ b/test/sql-tap/subquery.test.lua
@@ -352,33 +352,25 @@ test:do_execsql_test(
 test:do_test(
     "subquery-2.5.2",
     function()
-        -- In the expr "x IN (SELECT a FROM t3)" the RHS of the IN operator
-        -- has text affinity and the LHS has integer affinity.  The rule is
-        -- that we try to convert both sides to an integer before doing the
-        -- 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"
+        1, "Type mismatch: can not convert integer to string"
         -- </subquery-2.5.2>
     })
 
 test:do_test(
     "subquery-2.5.3.1",
     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"
+        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;

Re: [Tarantool-patches] [PATCH v1 1/2] sql: fix comparison in IN with list of values

[Mergen Imeev]

Sorry, I forgot about this comment. I changed the comment
in test. Diff below.

On Tue, Apr 28, 2020 at 12:53:34AM +0200, Vladislav Shpilevoy wrote:
> > 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
> > new file mode 100755
> > index 0000000..14ba7ad
> > --- /dev/null
> > +++ b/test/sql-tap/gh-4692-comparison-in-IN-operator.test.lua
> > @@ -0,0 +1,32 @@
> > +#!/usr/bin/env tarantool
> > +test = require("sqltester")
> > +test:plan(2)
> > +
> > +--
> > +-- If left value of IN and NOT IN operators is not a vector with
> > +-- length more that one, make sure that it cannot be compared to
> 
> that -> than.

Diff

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 a03b688..5e69805 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
@@ -3,9 +3,8 @@ test = require("sqltester")
 test:plan(11)
 
 --
--- If left value of IN and NOT IN operators is not a vector with
--- length more that one, make sure that it cannot be compared to
--- right values in case they are not comparable.
+-- Make sure that the left value of IN and NOT IN operators cannot
+-- be compared to the right value in case they are not comparable.
 --
 
 test:do_catchsql_test(

Re: [Tarantool-patches] [PATCH v1 1/2] sql: fix comparison in IN with list of values

[Vladislav Shpilevoy]

Hi! Thanks for the patch!

> 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 a03b688..5e69805 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
> @@ -3,9 +3,8 @@ test = require("sqltester")
>  test:plan(11)
>  
>  --
> --- If left value of IN and NOT IN operators is not a vector with
> --- length more that one, make sure that it cannot be compared to
> --- right values in case they are not comparable.
> +-- Make sure that the left value of IN and NOT IN operators cannot
> +-- be compared to the right value in case they are not comparable.
>  --

You change it in the second commit. But in the first commit it is still
'that' instead of 'than'.

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

[Vladislav Shpilevoy]

Thanks for the patch!

Why does this commit only make compile-time checks, and the
previous commit generates runtime OP_ApplyType?