[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)
®Free1);
r2 = sqlExprCodeTemp(pParse, pExpr->pRight,
®Free2);
+ 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,
®Free1);
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,
®Free1);
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