* [Tarantool-patches] [PATCH] sql: remove ApplyType opcode during <IN> process
@ 2020-02-29 13:27 Roman Khabibov
2020-03-02 11:52 ` Nikita Pettik
0 siblings, 1 reply; 2+ messages in thread
From: Roman Khabibov @ 2020-02-29 13:27 UTC (permalink / raw)
To: tarantool-patches
Don't apply type during <IN> processing.
---
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, {
-- <in3-3.3>
- 1, true
+ 1, false
-- </in3-3.3>
})
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, {
-- <subquery-2.5.2>
- "10.0"
-- </subquery-2.5.2>
})
@@ -378,7 +377,6 @@ test:do_test(
]]
end, {
-- <subquery-2.5.3.1>
- "10.0"
-- </subquery-2.5.3.1>
})
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..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)
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [Tarantool-patches] [PATCH] sql: remove ApplyType opcode during <IN> process
2020-02-29 13:27 [Tarantool-patches] [PATCH] sql: remove ApplyType opcode during <IN> process Roman Khabibov
@ 2020-03-02 11:52 ` Nikita Pettik
0 siblings, 0 replies; 2+ messages in thread
From: Nikita Pettik @ 2020-03-02 11:52 UTC (permalink / raw)
To: Roman Khabibov; +Cc: tarantool-patches
On 29 Feb 16:27, Roman Khabibov wrote:
> Don't apply type during <IN> 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, {
> -- <in3-3.3>
> - 1, true
> + 1, false
> -- </in3-3.3>
> })
>
> 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, {
> -- <subquery-2.5.2>
> - "10.0"
> -- </subquery-2.5.2>
> })
>
> @@ -378,7 +377,6 @@ test:do_test(
> ]]
> end, {
> -- <subquery-2.5.3.1>
> - "10.0"
> -- </subquery-2.5.3.1>
> })
>
> 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..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)
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-03-02 11:52 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-29 13:27 [Tarantool-patches] [PATCH] sql: remove ApplyType opcode during <IN> process Roman Khabibov
2020-03-02 11:52 ` Nikita Pettik
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox