From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 70D5324D50 for ; Thu, 17 Jan 2019 14:53:58 -0500 (EST) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 1p1CiyJixxkR for ; Thu, 17 Jan 2019 14:53:58 -0500 (EST) 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 turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 1EA212270E for ; Thu, 17 Jan 2019 14:53:57 -0500 (EST) Subject: [tarantool-patches] Re: [PATCH] sql: make IN operator stop ignoring type and collation References: <20190116133401.5844-1-korablev@tarantool.org> From: Vladislav Shpilevoy Message-ID: Date: Thu, 17 Jan 2019 22:53:46 +0300 MIME-Version: 1.0 In-Reply-To: <20190116133401.5844-1-korablev@tarantool.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org, Nikita Pettik LGTM On 16/01/2019 16:34, Nikita Pettik wrote: > SQLite discards type and collation of IN operator when it comes with > only one operand. This leads to different results of straight comparison > using '=' operator and IN: > > SELECT x FROM t1 WHERE x IN (1.0); > -- Result is empty set > SELECT x FROM t1 WHERE x = 1.0; > - - ['1'] > > Lets remove this strange ignorance and always take into consideration > types and collations of operands. > > Closes #3934 > --- > Branch: https://github.com/tarantool/tarantool/tree/np/gh-3934-IN-operator-fix > Issue: https://github.com/tarantool/tarantool/issues/3934 > > src/box/sql/expr.c | 4 ---- > src/box/sql/parse.y | 11 ----------- > src/box/sql/sqliteInt.h | 1 - > test/sql-tap/in4.test.lua | 8 ++++---- > test/sql-tap/tkt-9a8b09f8e6.test.lua | 7 ++++--- > 5 files changed, 8 insertions(+), 23 deletions(-) > > diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c > index b67b22c23..d83be5101 100644 > --- a/src/box/sql/expr.c > +++ b/src/box/sql/expr.c > @@ -73,8 +73,6 @@ char > sqlite3ExprAffinity(Expr * pExpr) > { > pExpr = sqlite3ExprSkipCollate(pExpr); > - if (pExpr->flags & EP_Generic) > - return 0; > uint8_t op = pExpr->op; > struct ExprList *el; > if (op == TK_REGISTER) > @@ -197,8 +195,6 @@ sql_expr_coll(Parse *parse, Expr *p, bool *is_explicit_coll, uint32_t *coll_id) > *coll_id = COLL_NONE; > while (p != NULL) { > int op = p->op; > - if (p->flags & EP_Generic) > - break; > if (op == TK_CAST || op == TK_UPLUS) { > p = p->pLeft; > continue; > diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y > index 50bb2ba01..0bcf41594 100644 > --- a/src/box/sql/parse.y > +++ b/src/box/sql/parse.y > @@ -1101,21 +1101,10 @@ expr(A) ::= expr(A) in_op(N) LP exprlist(Y) RP(E). [IN] { > ** > ** expr1 == ?1 > ** expr1 <> ?2 > - ** > - ** But, the RHS of the == or <> is marked with the EP_Generic flag > - ** so that it may not contribute to the computation of comparison > - ** affinity or the collating sequence to use for comparison. Otherwise, > - ** the semantics would be subtly different from IN or NOT IN. > */ > Expr *pRHS = Y->a[0].pExpr; > Y->a[0].pExpr = 0; > sql_expr_list_delete(pParse->db, Y); > - /* pRHS cannot be NULL because a malloc error would have been detected > - ** before now and control would have never reached this point */ > - if( ALWAYS(pRHS) ){ > - pRHS->flags &= ~EP_Collate; > - pRHS->flags |= EP_Generic; > - } > A.pExpr = sqlite3PExpr(pParse, N ? TK_NE : TK_EQ, A.pExpr, pRHS); > }else{ > A.pExpr = sqlite3PExpr(pParse, TK_IN, A.pExpr, 0); > diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h > index 7e16edc9a..ee24e0337 100644 > --- a/src/box/sql/sqliteInt.h > +++ b/src/box/sql/sqliteInt.h > @@ -2158,7 +2158,6 @@ struct Expr { > #define EP_DblQuoted 0x000040 /* token.z was originally in "..." */ > #define EP_InfixFunc 0x000080 /* True for an infix function: LIKE, etc */ > #define EP_Collate 0x000100 /* Tree contains a TK_COLLATE operator */ > -#define EP_Generic 0x000200 /* Ignore COLLATE or affinity on this tree */ > #define EP_IntValue 0x000400 /* Integer value contained in u.iValue */ > #define EP_xIsSelect 0x000800 /* x.pSelect is valid (otherwise x.pList is) */ > #define EP_Skip 0x001000 /* COLLATE, AS, or UNLIKELY */ > diff --git a/test/sql-tap/in4.test.lua b/test/sql-tap/in4.test.lua > index b029535af..c488cde45 100755 > --- a/test/sql-tap/in4.test.lua > +++ b/test/sql-tap/in4.test.lua > @@ -518,8 +518,8 @@ test:do_execsql_test( > }) > > -- MUST_WORK_TEST > --- Make sure that when "x IN (?)" is converted into "x==?" that collating > --- sequence and affinity computations do not get messed up. > +-- Make sure that when "x IN (?)" is converted into "x==?": > +-- type and collation sequence should be taken into consideration. > -- > test:do_execsql_test( > "in4-4.1", > @@ -571,7 +571,7 @@ test:do_execsql_test( > SELECT c FROM t4a WHERE a IN (b) ORDER BY c; > ]], { > -- > - 3 > + 1, 3 > -- > }) > > @@ -581,7 +581,7 @@ test:do_execsql_test( > SELECT c FROM t4a WHERE (a||'') IN (b) ORDER BY c; > ]], { > -- > - 3 > + 1, 3 > -- > }) > > diff --git a/test/sql-tap/tkt-9a8b09f8e6.test.lua b/test/sql-tap/tkt-9a8b09f8e6.test.lua > index 4d08cd657..996809afe 100755 > --- a/test/sql-tap/tkt-9a8b09f8e6.test.lua > +++ b/test/sql-tap/tkt-9a8b09f8e6.test.lua > @@ -99,7 +99,7 @@ test:do_execsql_test( > SELECT x FROM t1 WHERE x IN (1.0); > ]], { > -- <2.2> > - > + "1" > -- > }) > > @@ -239,7 +239,7 @@ test:do_execsql_test( > SELECT x FROM t2 WHERE '1.0' IN (x); > ]], { > -- <3.8> > - > + 1 > -- > }) > > @@ -309,7 +309,7 @@ test:do_execsql_test( > SELECT x FROM t3 WHERE '1' IN (x); > ]], { > -- <4.7> > - > + 1 > -- > }) > > @@ -519,6 +519,7 @@ test:do_execsql_test( > SELECT x, y FROM t5 WHERE '1.0' IN (x); > ]], { > -- <6.8> > + 1, "one", 1, "two", 1, "three", 1, "four" > -- > }) > >