From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 2AF1B46970E for ; Fri, 7 Feb 2020 14:34:44 +0300 (MSK) From: Roman Khabibov Date: Fri, 7 Feb 2020 14:34:40 +0300 Message-Id: <20200207113440.79470-1-roman.habibov@tarantool.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH] sql: fix bug type mismatch error List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@dev.tarantool.org Ignore type mismatch error in the certain VDBE operations, if they are used to process 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( -- }) -test:do_catchsql_test( +test:do_execsql_test( "tkt-80e031a00f.27", [[ SELECT 'hello' IN t1 ]], { -- - 1, 'Type mismatch: can not convert hello to integer' + false -- }) -test:do_catchsql_test( +test:do_execsql_test( "tkt-80e031a00f.28", [[ SELECT 'hello' NOT IN t1 ]], { -- - 1, 'Type mismatch: can not convert hello to integer' + true -- }) @@ -380,23 +380,23 @@ test:do_execsql_test( -- }) -test:do_catchsql_test( +test:do_execsql_test( "tkt-80e031a00f.31", [[ SELECT x'303132' IN t1 ]], { -- - 1, 'Type mismatch: can not convert varbinary to integer' + false -- }) -test:do_catchsql_test( +test:do_execsql_test( "tkt-80e031a00f.32", [[ SELECT x'303132' NOT IN t1 ]], { -- - 1, 'Type mismatch: can not convert varbinary to integer' + true -- }) 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)