* [tarantool-patches] [PATCH] sql: make IN operator stop ignoring type and collation
@ 2019-01-16 13:34 Nikita Pettik
2019-01-17 19:53 ` [tarantool-patches] " Vladislav Shpilevoy
2019-01-24 13:56 ` Kirill Yukhin
0 siblings, 2 replies; 3+ messages in thread
From: Nikita Pettik @ 2019-01-16 13:34 UTC (permalink / raw)
To: tarantool-patches; +Cc: v.shpilevoy, Nikita Pettik
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;
]], {
-- <in4-4.5>
- 3
+ 1, 3
-- </in4-4.5>
})
@@ -581,7 +581,7 @@ test:do_execsql_test(
SELECT c FROM t4a WHERE (a||'') IN (b) ORDER BY c;
]], {
-- <in4-4.6>
- 3
+ 1, 3
-- </in4-4.6>
})
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"
-- </2.2>
})
@@ -239,7 +239,7 @@ test:do_execsql_test(
SELECT x FROM t2 WHERE '1.0' IN (x);
]], {
-- <3.8>
-
+ 1
-- </3.8>
})
@@ -309,7 +309,7 @@ test:do_execsql_test(
SELECT x FROM t3 WHERE '1' IN (x);
]], {
-- <4.7>
-
+ 1
-- </4.7>
})
@@ -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"
-- </6.8>
})
--
2.15.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* [tarantool-patches] Re: [PATCH] sql: make IN operator stop ignoring type and collation
2019-01-16 13:34 [tarantool-patches] [PATCH] sql: make IN operator stop ignoring type and collation Nikita Pettik
@ 2019-01-17 19:53 ` Vladislav Shpilevoy
2019-01-24 13:56 ` Kirill Yukhin
1 sibling, 0 replies; 3+ messages in thread
From: Vladislav Shpilevoy @ 2019-01-17 19:53 UTC (permalink / raw)
To: tarantool-patches, 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;
> ]], {
> -- <in4-4.5>
> - 3
> + 1, 3
> -- </in4-4.5>
> })
>
> @@ -581,7 +581,7 @@ test:do_execsql_test(
> SELECT c FROM t4a WHERE (a||'') IN (b) ORDER BY c;
> ]], {
> -- <in4-4.6>
> - 3
> + 1, 3
> -- </in4-4.6>
> })
>
> 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"
> -- </2.2>
> })
>
> @@ -239,7 +239,7 @@ test:do_execsql_test(
> SELECT x FROM t2 WHERE '1.0' IN (x);
> ]], {
> -- <3.8>
> -
> + 1
> -- </3.8>
> })
>
> @@ -309,7 +309,7 @@ test:do_execsql_test(
> SELECT x FROM t3 WHERE '1' IN (x);
> ]], {
> -- <4.7>
> -
> + 1
> -- </4.7>
> })
>
> @@ -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"
> -- </6.8>
> })
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* [tarantool-patches] Re: [PATCH] sql: make IN operator stop ignoring type and collation
2019-01-16 13:34 [tarantool-patches] [PATCH] sql: make IN operator stop ignoring type and collation Nikita Pettik
2019-01-17 19:53 ` [tarantool-patches] " Vladislav Shpilevoy
@ 2019-01-24 13:56 ` Kirill Yukhin
1 sibling, 0 replies; 3+ messages in thread
From: Kirill Yukhin @ 2019-01-24 13:56 UTC (permalink / raw)
To: tarantool-patches; +Cc: v.shpilevoy, Nikita Pettik
Hello,
On 16 Jan 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
I've checked your patch into 2.1 branch.
--
Regards, Kirill Yukhin
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-01-24 13:56 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-16 13:34 [tarantool-patches] [PATCH] sql: make IN operator stop ignoring type and collation Nikita Pettik
2019-01-17 19:53 ` [tarantool-patches] " Vladislav Shpilevoy
2019-01-24 13:56 ` Kirill Yukhin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox