[Tarantool-patches] [PATCH] sql: fix bug <IN> type mismatch error

Roman Khabibov roman.habibov at tarantool.org
Fri Feb 7 14:34:40 MSK 2020


Ignore type mismatch error in the certain VDBE operations, if they
are used to process <IN> predicate.

Closes #4692
---
Branch: https://github.com/tarantool/tarantool/pull/new/romanhabibov/gh-4692-in-bug
Issue: https://github.com/tarantool/tarantool/issues/4692
 src/box/sql/expr.c                   | 27 ++++++++----
 src/box/sql/parse.y                  |  2 +
 src/box/sql/sqlInt.h                 |  7 ++++
 src/box/sql/vdbe.c                   |  9 ++--
 test/sql-tap/tkt-80e031a00f.test.lua | 16 +++----
 test/sql/boolean.result              | 42 +++++++++++++------
 test/sql/gh-4692-in-bug.result       | 62 ++++++++++++++++++++++++++++
 test/sql/gh-4692-in-bug.test.lua     | 17 ++++++++
 8 files changed, 151 insertions(+), 31 deletions(-)
 create mode 100644 test/sql/gh-4692-in-bug.result
 create mode 100755 test/sql/gh-4692-in-bug.test.lua

diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index bc2182446..ee72b90d9 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -425,12 +425,12 @@ expr_cmp_mutual_type(struct Expr *pExpr)
  * Return the P5 value that should be used for a binary comparison
  * opcode (OP_Eq, OP_Ge etc.) used to compare pExpr1 and pExpr2.
  */
-static u8
+static u16
 binaryCompareP5(Expr * pExpr1, Expr * pExpr2, int jumpIfNull)
 {
 	enum field_type lhs = sql_expr_type(pExpr2);
 	enum field_type rhs = sql_expr_type(pExpr1);
-	u8 type_mask = sql_type_result(rhs, lhs) | (u8) jumpIfNull;
+	u16 type_mask = sql_type_result(rhs, lhs) | (u16) jumpIfNull;
 	return type_mask;
 }
 
@@ -515,7 +515,7 @@ codeCompare(Parse * pParse,	/* The parsing (and code generating) context */
 	int p5 = binaryCompareP5(pLeft, pRight, jumpIfNull);
 	int addr = sqlVdbeAddOp4(pParse->pVdbe, opcode, in2, dest, in1,
 				     (void *)coll, P4_COLLSEQ);
-	sqlVdbeChangeP5(pParse->pVdbe, (u8) p5);
+	sqlVdbeChangeP5(pParse->pVdbe, p5);
 	return addr;
 }
 
@@ -736,6 +736,8 @@ codeVectorCompare(Parse * pParse,	/* Code generator context */
 
 	u8 opx;
 	u8 p5 = SQL_STOREP2;
+	if ((pExpr->flags & EP_IgnoreMismatch) != 0)
+		p5 |= SQL_IGNORE_MISMATCH;
 	if (op == TK_LE)
 		opx = TK_LT;
 	else if (op == TK_GE)
@@ -3140,16 +3142,16 @@ sqlExprCodeIN(Parse * pParse,	/* Parsing and code generating context */
 						  (void *)coll, P4_COLLSEQ);
 				VdbeCoverageIf(v, ii < pList->nExpr - 1);
 				VdbeCoverageIf(v, ii == pList->nExpr - 1);
-				sqlVdbeChangeP5(v, zAff[0]);
+				sqlVdbeChangeP5(v, zAff[0] |
+						SQL_IGNORE_MISMATCH);
 			} else {
 				assert(destIfNull == destIfFalse);
 				sqlVdbeAddOp4(v, OP_Ne, rLhs, destIfFalse,
 						  r2, (void *)coll,
 						  P4_COLLSEQ);
 				VdbeCoverage(v);
-				sqlVdbeChangeP5(v,
-						    zAff[0] |
-						    SQL_JUMPIFNULL);
+				sqlVdbeChangeP5(v, zAff[0] | SQL_JUMPIFNULL |
+						SQL_IGNORE_MISMATCH);
 			}
 			sqlReleaseTempReg(pParse, regToFree);
 		}
@@ -3187,6 +3189,7 @@ sqlExprCodeIN(Parse * pParse,	/* Parsing and code generating context */
 	zAff[nVector] = field_type_MAX;
 	sqlVdbeAddOp4(v, OP_ApplyType, rLhs, nVector, 0, (char*)zAff,
 			  P4_DYNAMIC);
+	sqlVdbeChangeP5(v, SQL_IGNORE_MISMATCH);
 	/*
 	 * zAff will be freed at the end of VDBE execution, since
 	 * it was passed with P4_DYNAMIC flag.
@@ -3252,6 +3255,7 @@ sqlExprCodeIN(Parse * pParse,	/* Parsing and code generating context */
 		sqlVdbeAddOp3(v, OP_Column, pExpr->iTable, aiMap[i], r3);
 		sqlVdbeAddOp4(v, OP_Ne, rLhs + i, destNotNull, r3,
 				  (void *)pColl, P4_COLLSEQ);
+		sqlVdbeChangeP5(v, SQL_IGNORE_MISMATCH);
 		VdbeCoverage(v);
 		sqlReleaseTempReg(pParse, r3);
 	}
@@ -3824,8 +3828,11 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
 							 &regFree1);
 				r2 = sqlExprCodeTemp(pParse, pExpr->pRight,
 							 &regFree2);
+				int flags = SQL_STOREP2;
+				if ((pExpr->flags & EP_IgnoreMismatch) != 0)
+					flags |= SQL_IGNORE_MISMATCH;
 				codeCompare(pParse, pLeft, pExpr->pRight, op,
-					    r1, r2, inReg, SQL_STOREP2);
+					    r1, r2, inReg, flags);
 				assert(TK_LT == OP_Lt);
 				testcase(op == OP_Lt);
 				VdbeCoverageIf(v, op == OP_Lt);
@@ -4755,6 +4762,8 @@ sqlExprIfTrue(Parse * pParse, Expr * pExpr, int dest, int jumpIfNull)
 			if (sqlExprIsVector(pExpr->pLeft))
 				goto default_expr;
 			testcase(jumpIfNull == 0);
+			if ((pExpr->flags & EP_IgnoreMismatch) != 0)
+				jumpIfNull |= SQL_IGNORE_MISMATCH;
 			r1 = sqlExprCodeTemp(pParse, pExpr->pLeft,
 						 &regFree1);
 			r2 = sqlExprCodeTemp(pParse, pExpr->pRight,
@@ -4948,6 +4957,8 @@ sqlExprIfFalse(Parse * pParse, Expr * pExpr, int dest, int jumpIfNull)
 			if (sqlExprIsVector(pExpr->pLeft))
 				goto default_expr;
 			testcase(jumpIfNull == 0);
+			if ((pExpr->flags & EP_IgnoreMismatch) != 0)
+				jumpIfNull |= SQL_IGNORE_MISMATCH;
 			r1 = sqlExprCodeTemp(pParse, pExpr->pLeft,
 						 &regFree1);
 			r2 = sqlExprCodeTemp(pParse, pExpr->pRight,
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index cfe1c0012..679758d4c 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -1383,6 +1383,8 @@ expr(A) ::= expr(A) in_op(N) LP exprlist(Y) RP(E). [IN] {
     Y->a[0].pExpr = 0;
     sql_expr_list_delete(pParse->db, Y);
     A.pExpr = sqlPExpr(pParse, N ? TK_NE : TK_EQ, A.pExpr, pRHS);
+    if (A.pExpr != NULL)
+      A.pExpr->flags |= EP_IgnoreMismatch;
   }else{
     A.pExpr = sqlPExpr(pParse, TK_IN, A.pExpr, 0);
     if( A.pExpr ){
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index d1fcf4761..1a9d6961d 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -1318,6 +1318,11 @@ enum trim_side_mask {
 #define SQL_KEEPNULL     0x40	/* Used by vector == or <> */
 #define SQL_NULLEQ       0x80	/* NULL=NULL */
 #define SQL_NOTNULL      0x90	/* Assert that operands are never NULL */
+/*
+ * Don't throw ER_SQL_TYPE_MISMATCH when compared types are
+ * different.
+ */
+#define SQL_IGNORE_MISMATCH 0x100
 
 /**
  * Return logarithm of tuple count in space.
@@ -1632,6 +1637,8 @@ struct Expr {
 #define EP_Leaf      0x800000	/* Expr.pLeft, .pRight, .u.pSelect all NULL */
 /** Expression is system-defined. */
 #define EP_System    0x1000000
+/** Ignore type mismatch error. */
+#define EP_IgnoreMismatch 0x2000000
 
 /*
  * Combinations of two or more EP_* flags
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index eab74db4a..b201440d6 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -2191,7 +2191,8 @@ case OP_Ge: {             /* same as TK_GE, jump, in1, in3 */
 		 */
 		int type_arg1 = flags1 & (MEM_Bool | MEM_Blob);
 		int type_arg3 = flags3 & (MEM_Bool | MEM_Blob);
-		if (type_arg1 != type_arg3) {
+		if (type_arg1 != type_arg3 &&
+		    (pOp->p5 & SQL_IGNORE_MISMATCH) == 0) {
 			char *inconsistent_type = type_arg1 != 0 ?
 						  mem_type_to_str(pIn3) :
 						  mem_type_to_str(pIn1);
@@ -2213,7 +2214,8 @@ case OP_Ge: {             /* same as TK_GE, jump, in1, in3 */
 					flags3 = pIn3->flags;
 				}
 				if ((flags3 & MEM_Str) == MEM_Str) {
-					if (mem_apply_numeric_type(pIn3) != 0) {
+					if (mem_apply_numeric_type(pIn3) != 0 &&
+					    (pOp->p5 & SQL_IGNORE_MISMATCH) == 0) {
 						diag_set(ClientError,
 							 ER_SQL_TYPE_MISMATCH,
 							 sql_value_to_diag_str(pIn3),
@@ -2779,7 +2781,8 @@ case OP_ApplyType: {
 	while((type = *(types++)) != field_type_MAX) {
 		assert(pIn1 <= &p->aMem[(p->nMem+1 - p->nCursor)]);
 		assert(memIsValid(pIn1));
-		if (mem_apply_type(pIn1, type) != 0) {
+		if (mem_apply_type(pIn1, type) != 0 &&
+		    (pOp->p5 & SQL_IGNORE_MISMATCH) == 0) {
 			diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
 				 sql_value_to_diag_str(pIn1),
 				 field_type_strs[type]);
diff --git a/test/sql-tap/tkt-80e031a00f.test.lua b/test/sql-tap/tkt-80e031a00f.test.lua
index a0e6539e0..8553a15a8 100755
--- a/test/sql-tap/tkt-80e031a00f.test.lua
+++ b/test/sql-tap/tkt-80e031a00f.test.lua
@@ -340,23 +340,23 @@ test:do_execsql_test(
         -- </tkt-80e031a00f.26>
     })
 
-test:do_catchsql_test(
+test:do_execsql_test(
     "tkt-80e031a00f.27",
     [[
         SELECT 'hello' IN t1
     ]], {
         -- <tkt-80e031a00f.27>
-        1, 'Type mismatch: can not convert hello to integer'
+        false
         -- </tkt-80e031a00f.27>
     })
 
-test:do_catchsql_test(
+test:do_execsql_test(
     "tkt-80e031a00f.28",
     [[
         SELECT 'hello' NOT IN t1
     ]], {
         -- <tkt-80e031a00f.28>
-        1, 'Type mismatch: can not convert hello to integer'
+        true
         -- </tkt-80e031a00f.28>
     })
 
@@ -380,23 +380,23 @@ test:do_execsql_test(
         -- </tkt-80e031a00f.30>
     })
 
-test:do_catchsql_test(
+test:do_execsql_test(
     "tkt-80e031a00f.31",
     [[
         SELECT x'303132' IN t1
     ]], {
         -- <tkt-80e031a00f.31>
-        1, 'Type mismatch: can not convert varbinary to integer'
+        false
         -- </tkt-80e031a00f.31>
     })
 
-test:do_catchsql_test(
+test:do_execsql_test(
     "tkt-80e031a00f.32",
     [[
         SELECT x'303132' NOT IN t1
     ]], {
         -- <tkt-80e031a00f.32>
-        1, 'Type mismatch: can not convert varbinary to integer'
+        true
         -- </tkt-80e031a00f.32>
     })
 
diff --git a/test/sql/boolean.result b/test/sql/boolean.result
index 7769d0cb3..8b3869879 100644
--- a/test/sql/boolean.result
+++ b/test/sql/boolean.result
@@ -3871,13 +3871,19 @@ SELECT false IN (0, 1, 2, 3);
  | ...
 SELECT true IN (SELECT b FROM t7);
  | ---
- | - null
- | - 'Type mismatch: can not convert TRUE to integer'
+ | - metadata:
+ |   - name: true IN (SELECT b FROM t7)
+ |     type: boolean
+ |   rows:
+ |   - [false]
  | ...
 SELECT false IN (SELECT b FROM t7);
  | ---
- | - null
- | - 'Type mismatch: can not convert FALSE to integer'
+ | - metadata:
+ |   - name: false IN (SELECT b FROM t7)
+ |     type: boolean
+ |   rows:
+ |   - [false]
  | ...
 SELECT a1, a1 IN (0, 1, 2, 3) FROM t6
  | ---
@@ -5032,23 +5038,35 @@ 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'
+ | - metadata:
+ |   - name: true IN (SELECT c FROM t8)
+ |     type: boolean
+ |   rows:
+ |   - [false]
  | ...
 SELECT false IN (SELECT c FROM t8);
  | ---
- | - null
- | - 'Type mismatch: can not convert FALSE to number'
+ | - metadata:
+ |   - name: false IN (SELECT c FROM t8)
+ |     type: boolean
+ |   rows:
+ |   - [false]
  | ...
 SELECT a1 IN (SELECT c FROM t8) FROM t6 LIMIT 1;
  | ---
- | - null
- | - 'Type mismatch: can not convert FALSE to number'
+ | - metadata:
+ |   - name: a1 IN (SELECT c FROM t8)
+ |     type: boolean
+ |   rows:
+ |   - [false]
  | ...
 SELECT a2 IN (SELECT c FROM t8) FROM t6 LIMIT 1;
  | ---
- | - null
- | - 'Type mismatch: can not convert TRUE to number'
+ | - metadata:
+ |   - name: a2 IN (SELECT c FROM t8)
+ |     type: boolean
+ |   rows:
+ |   - [false]
  | ...
 
 SELECT true BETWEEN 0.1 and 9.9;
diff --git a/test/sql/gh-4692-in-bug.result b/test/sql/gh-4692-in-bug.result
new file mode 100644
index 000000000..b0a7db6c5
--- /dev/null
+++ b/test/sql/gh-4692-in-bug.result
@@ -0,0 +1,62 @@
+-- test-run result file version 2
+test_run = require('test_run').new()
+ | ---
+ | ...
+engine = test_run:get_cfg('engine')
+ | ---
+ | ...
+
+--
+-- gh-4692: Make sure that there is no type mismatch error.
+--
+box.execute("CREATE TABLE t1(a INT PRIMARY KEY);")
+ | ---
+ | - row_count: 1
+ | ...
+box.execute("SELECT a FROM t1 WHERE a IN (a+8,true);")
+ | ---
+ | - metadata:
+ |   - name: A
+ |     type: integer
+ |   rows: []
+ | ...
+box.execute("SELECT true IN (1);")
+ | ---
+ | - metadata:
+ |   - name: true IN (1)
+ |     type: boolean
+ |   rows:
+ |   - [false]
+ | ...
+box.execute("SELECT true IN (VALUES(1));")
+ | ---
+ | - metadata:
+ |   - name: true IN (VALUES(1))
+ |     type: boolean
+ |   rows:
+ |   - [false]
+ | ...
+box.execute("SELECT true IN (1, 2);")
+ | ---
+ | - metadata:
+ |   - name: true IN (1, 2)
+ |     type: boolean
+ |   rows:
+ |   - [false]
+ | ...
+box.execute("SELECT true IN (VALUES(1), (2), ('3'));")
+ | ---
+ | - metadata:
+ |   - name: true IN (VALUES(1), (2), ('3'))
+ |     type: boolean
+ |   rows:
+ |   - [false]
+ | ...
+
+--
+-- Cleanup.
+--
+box.execute('DROP TABLE t1;')
+ | ---
+ | - row_count: 1
+ | ...
diff --git a/test/sql/gh-4692-in-bug.test.lua b/test/sql/gh-4692-in-bug.test.lua
new file mode 100755
index 000000000..fa52615a6
--- /dev/null
+++ b/test/sql/gh-4692-in-bug.test.lua
@@ -0,0 +1,17 @@
+test_run = require('test_run').new()
+engine = test_run:get_cfg('engine')
+
+--
+-- gh-4692: Make sure that there is no type mismatch error.
+--
+box.execute("CREATE TABLE t1(a INT PRIMARY KEY);")
+box.execute("SELECT a FROM t1 WHERE a IN (a+8,true);")
+box.execute("SELECT true IN (1);")
+box.execute("SELECT true IN (VALUES(1));")
+box.execute("SELECT true IN (1, 2);")
+box.execute("SELECT true IN (VALUES(1), (2), ('3'));")
+
+--
+-- Cleanup.
+--
+box.execute('DROP TABLE t1;')
-- 
2.21.0 (Apple Git-122)



More information about the Tarantool-patches mailing list