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 4DC65469719 for ; Mon, 2 Mar 2020 14:52:23 +0300 (MSK) Date: Mon, 2 Mar 2020 14:52:22 +0300 From: Nikita Pettik Message-ID: <20200302115222.GB43956@tarantool.org> References: <20200229132710.68778-1-roman.habibov@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20200229132710.68778-1-roman.habibov@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH] sql: remove ApplyType opcode during process List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Roman Khabibov Cc: tarantool-patches@dev.tarantool.org On 29 Feb 16:27, Roman Khabibov wrote: > Don't apply type during processing. > --- Please, provide justification and explanation. I can't provide review otherwise. > Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/gh-4692-in-bug > > src/box/sql/expr.c | 18 +++--------------- > test/sql-tap/in3.test.lua | 2 +- > test/sql-tap/subquery.test.lua | 2 -- > test/sql-tap/tkt-80e031a00f.test.lua | 16 ++++++++-------- > test/sql/boolean.result | 12 ++++++------ > 5 files changed, 18 insertions(+), 32 deletions(-) > > diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c > index bc2182446..33ad57c67 100644 > --- a/src/box/sql/expr.c > +++ b/src/box/sql/expr.c > @@ -3058,8 +3058,6 @@ sqlExprCodeIN(Parse * pParse, /* Parsing and code generating context */ > pLeft = pExpr->pLeft; > if (sqlExprCheckIN(pParse, pExpr)) > return; > - /* Type sequence for comparisons. */ > - enum field_type *zAff = expr_in_type(pParse, pExpr); > nVector = sqlExprVectorSize(pExpr->pLeft); > aiMap = > (int *)sqlDbMallocZero(pParse->db, > @@ -3140,16 +3138,15 @@ 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, sql_expr_type(pLeft)); > } 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, sql_expr_type(pLeft) | > + SQL_JUMPIFNULL); > } > sqlReleaseTempReg(pParse, regToFree); > } > @@ -3184,14 +3181,6 @@ sqlExprCodeIN(Parse * pParse, /* Parsing and code generating context */ > * of the RHS using the LHS as a probe. If found, the result is > * true. > */ > - zAff[nVector] = field_type_MAX; > - sqlVdbeAddOp4(v, OP_ApplyType, rLhs, nVector, 0, (char*)zAff, > - P4_DYNAMIC); > - /* > - * zAff will be freed at the end of VDBE execution, since > - * it was passed with P4_DYNAMIC flag. > - */ > - zAff = NULL; > if (destIfFalse == destIfNull) { > /* Combine Step 3 and Step 5 into a single opcode */ > sqlVdbeAddOp4Int(v, OP_NotFound, pExpr->iTable, > @@ -3277,7 +3266,6 @@ sqlExprCodeIN(Parse * pParse, /* Parsing and code generating context */ > VdbeComment((v, "end IN expr")); > sqlExprCodeIN_oom_error: > sqlDbFree(pParse->db, aiMap); > - sqlDbFree(pParse->db, zAff); > } > > /* > diff --git a/test/sql-tap/in3.test.lua b/test/sql-tap/in3.test.lua > index e29db9d93..72782b54f 100755 > --- a/test/sql-tap/in3.test.lua > +++ b/test/sql-tap/in3.test.lua > @@ -330,7 +330,7 @@ test:do_test( > return exec_neph(" SELECT x IN (SELECT b FROM t1) FROM t2 ") > end, { > -- > - 1, true > + 1, false > -- > }) > > diff --git a/test/sql-tap/subquery.test.lua b/test/sql-tap/subquery.test.lua > index 6bedf5879..c4619e8f5 100755 > --- a/test/sql-tap/subquery.test.lua > +++ b/test/sql-tap/subquery.test.lua > @@ -363,7 +363,6 @@ test:do_test( > ]] > end, { > -- > - "10.0" > -- > }) > > @@ -378,7 +377,6 @@ test:do_test( > ]] > end, { > -- > - "10.0" > -- > }) > > 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..0abd88915 100644 > --- a/test/sql/boolean.result > +++ b/test/sql/boolean.result > @@ -3872,12 +3872,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 unsigned to boolean' > | ... > SELECT false IN (SELECT b FROM t7); > | --- > | - null > - | - 'Type mismatch: can not convert FALSE to integer' > + | - 'Type mismatch: can not convert unsigned to boolean' > | ... > SELECT a1, a1 IN (0, 1, 2, 3) FROM t6 > | --- > @@ -5033,22 +5033,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 real to boolean' > | ... > SELECT false IN (SELECT c FROM t8); > | --- > | - null > - | - 'Type mismatch: can not convert FALSE to number' > + | - 'Type mismatch: can not convert real 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 real 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 real to boolean' > | ... > > SELECT true BETWEEN 0.1 and 9.9; > -- > 2.21.0 (Apple Git-122) >