[tarantool-patches] [PATCH v1 1/1] sql: disallow returning many rows from subselect
Kirill Shcherbatov
kshcherbatov at tarantool.org
Mon Jun 18 14:55:03 MSK 2018
Branch: http://github.com/tarantool/tarantool/tree/kshch/gh-2366-whith-select-subquery
Issue: https://github.com/tarantool/tarantool/issues/2366
To follow ANSI SQL standard we should dissallow returning
multiple rows from subselects. To achieve this goal we have
introduced some special Select SF_SingleRow flag that indicates
the case of subselect having no client-defined LIMIT 1 to patch
system implicit LIMIT 1 to be LIMIT 2 and generate some extra
bytecode to HALT execution on reaching this restrict.
The place of patching LIMIT expression iValue is a very special:
this showld be done after simplifying epression tree(as this
restrict could become useless), but before generating related
bytecode.
Resolves #2366
---
src/box/sql/expr.c | 18 ++++++++++++++----
src/box/sql/select.c | 31 +++++++++++++++++++++++++++++--
src/box/sql/sqliteInt.h | 2 ++
test/sql-tap/aggnested.test.lua | 2 +-
test/sql-tap/e_expr.test.lua | 38 +++++++++++++++++++-------------------
test/sql-tap/misc5.test.lua | 15 ++++++---------
test/sql-tap/selectA.test.lua | 4 ++--
test/sql-tap/subquery.test.lua | 14 +++++++-------
test/sql-tap/subquery2.test.lua | 4 ++--
test/sql-tap/subselect.test.lua | 32 ++++++++++++++++++++++++++++----
test/sql-tap/with1.test.lua | 2 +-
11 files changed, 111 insertions(+), 51 deletions(-)
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index a69d38b..a22548c 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -2890,10 +2890,20 @@ sqlite3CodeSubselect(Parse * pParse, /* Parsing context */
dest.iSDParm);
VdbeComment((v, "Init EXISTS result"));
}
- sql_expr_delete(pParse->db, pSel->pLimit, false);
- pSel->pLimit = sqlite3ExprAlloc(pParse->db, TK_INTEGER,
- &sqlite3IntTokens[1],
- 0);
+ if (pSel->pLimit == NULL ||
+ !(pSel->pLimit->flags & EP_IntValue) ||
+ pSel->pLimit->u.iValue != 1) {
+ sql_expr_delete(pParse->db, pSel->pLimit, false);
+ pSel->pLimit = sqlite3ExprAlloc(pParse->db, TK_INTEGER,
+ &sqlite3IntTokens[1],
+ 0);
+ pSel->selFlags |= SF_SingleRow;
+ } else {
+ /*
+ * Customer LIMIT 1 in subquery
+ * should be kept.
+ */
+ }
pSel->iLimit = 0;
pSel->selFlags &= ~SF_MultiValue;
if (sqlite3Select(pParse, pSel, &dest)) {
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index 4b5ba4d..7b0c335 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -5665,6 +5665,21 @@ sqlite3Select(Parse * pParse, /* The parser context */
if ((p->selFlags & SF_FixedLimit) == 0) {
p->nSelectRow = 320; /* 4 billion rows */
}
+ bool single_row_assert =
+ (p->pLimit != NULL && p->selFlags & SF_SingleRow);
+ if (single_row_assert) {
+ /*
+ * We have to change synthetic LIMIT constraint
+ * to 2 here, because it should be set before
+ * computeLimitRegisters call and operations
+ * on tree, but after simplifying tree a bit
+ * earlier.
+ * Checks bytecode is generated in the end of
+ * this function.
+ */
+ assert(p->pLimit->flags & EP_IntValue);
+ p->pLimit->u.iValue = 2;
+ }
computeLimitRegisters(pParse, p, iEnd);
if (p->iLimit == 0 && sSort.addrSortIndex >= 0) {
sqlite3VdbeChangeOpcode(v, sSort.addrSortIndex, OP_SorterOpen);
@@ -6220,8 +6235,20 @@ sqlite3Select(Parse * pParse, /* The parser context */
generateSortTail(pParse, p, &sSort, pEList->nExpr, pDest);
}
- /* Jump here to skip this query
- */
+ /* Generate code that prevent returning multiple rows. */
+ assert(single_row_assert ==
+ (p->pLimit != NULL && p->selFlags & SF_SingleRow));
+ if (single_row_assert) {
+ int r1 = sqlite3GetTempReg(pParse);
+ sqlite3VdbeAddOp2(v, OP_Integer, 0, r1);
+ sqlite3VdbeAddOp3(v, OP_Ne, r1, iEnd, p->iLimit);
+ const char *error = "SELECT subquery returned more than 1 row";
+ sqlite3VdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR,
+ ON_CONFLICT_ACTION_FAIL, 0,
+ error, P4_STATIC);
+ sqlite3ReleaseTempReg(pParse, r1);
+ }
+ /* Jump here to skip this query. */
sqlite3VdbeResolveLabel(v, iEnd);
/* The SELECT has been coded. If there is an error in the Parse structure,
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index fc6b858..01642a4 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -2725,6 +2725,8 @@ struct Select {
#define SF_FixedLimit 0x04000 /* nSelectRow set by a constant LIMIT */
#define SF_MaybeConvert 0x08000 /* Need convertCompoundSelectToSubquery() */
#define SF_Converted 0x10000 /* By convertCompoundSelectToSubquery() */
+/** Abort subquery if its output contain more than one row. */
+#define SF_SingleRow 0x40000
/*
* The results of a SELECT can be distributed in several ways, as defined
diff --git a/test/sql-tap/aggnested.test.lua b/test/sql-tap/aggnested.test.lua
index e690033..627abdd 100755
--- a/test/sql-tap/aggnested.test.lua
+++ b/test/sql-tap/aggnested.test.lua
@@ -28,7 +28,7 @@ test:do_execsql_test(
INSERT INTO t1 VALUES(1), (2), (3);
CREATE TABLE t2(b1 INTEGER PRIMARY KEY);
INSERT INTO t2 VALUES(4), (5);
- SELECT (SELECT group_concat(a1,'x') FROM t2) FROM t1;
+ SELECT (SELECT group_concat(a1,'x') FROM t2 LIMIT 1) FROM t1;
]],
{
-- <aggnested-1.1>
diff --git a/test/sql-tap/e_expr.test.lua b/test/sql-tap/e_expr.test.lua
index 13d3a96..8058cb2 100755
--- a/test/sql-tap/e_expr.test.lua
+++ b/test/sql-tap/e_expr.test.lua
@@ -3382,11 +3382,11 @@ test:do_execsql_test(
-- more rows, then the EXISTS operator evaluates to 1.
--
local data = {
- {1, "EXISTS ( SELECT a FROM t1 )"},
- {2, "EXISTS ( SELECT b FROM t1 )"},
+ {1, "EXISTS ( SELECT a FROM t1 LIMIT 1 )"},
+ {2, "EXISTS ( SELECT b FROM t1 LIMIT 1 )"},
{3, "EXISTS ( SELECT 24 )"},
{4, "EXISTS ( SELECT NULL )"},
- {5, "EXISTS ( SELECT a FROM t1 WHERE a IS NULL )"},
+ {5, "EXISTS ( SELECT a FROM t1 WHERE a IS NULL LIMIT 1 )"},
}
for _, val in ipairs(data) do
local tn = val[1]
@@ -3412,13 +3412,13 @@ end
-- no effect on the results of the EXISTS operator.
--
data = {
- {1, "EXISTS ( SELECT a,b FROM t1 )", 1},
- {2, "EXISTS ( SELECT a,b, a,b, a,b FROM t1 )", 1},
+ {1, "EXISTS ( SELECT a,b FROM t1 LIMIT 1 )", 1},
+ {2, "EXISTS ( SELECT a,b, a,b, a,b FROM t1 LIMIT 1 )", 1},
{3, "EXISTS ( SELECT 24, 25 )", 1},
{4, "EXISTS ( SELECT NULL, NULL, NULL )", 1},
- {5, "EXISTS ( SELECT a,b,a||b FROM t1 WHERE a IS NULL )", 1},
- {6, "EXISTS ( SELECT a, a FROM t1 WHERE 0)", 0},
- {7, "EXISTS ( SELECT b, b, a FROM t1 WHERE a = 5)", 0},
+ {5, "EXISTS ( SELECT a,b,a||b FROM t1 WHERE a IS NULL LIMIT 1 )", 1},
+ {6, "EXISTS ( SELECT a, a FROM t1 WHERE 0 LIMIT 1 )", 0},
+ {7, "EXISTS ( SELECT b, b, a FROM t1 WHERE a = 5 LIMIT 1 )", 0},
{8, "EXISTS ( SELECT 24, 46, 89 WHERE 0)", 0},
{9, "EXISTS ( SELECT NULL, NULL WHERE 1=2)", 0},
}
@@ -3433,7 +3433,7 @@ end
--
data = {
{1, "EXISTS (SELECT 'not null')", "EXISTS (SELECT NULL)"},
- {2, "EXISTS (SELECT NULL FROM t1)", "EXISTS (SELECT 'bread' FROM t1)"},
+ {2, "EXISTS (SELECT NULL FROM t1 LIMIT 1)", "EXISTS (SELECT 'bread' FROM t1 LIMIT 1)"},
}
for _, val in ipairs(data) do
@@ -3474,7 +3474,7 @@ test:do_execsql_test(
do_expr_test("e_expr-35.1.1", " (SELECT 35) ", "integer", 35)
do_expr_test("e_expr-35.1.2", " (SELECT NULL) ", "null", "")
do_expr_test("e_expr-35.1.3", " (SELECT count(*) FROM t22) ", "integer", 3)
-do_expr_test("e_expr-35.1.4", " (SELECT 4 FROM t22) ", "integer", 4)
+do_expr_test("e_expr-35.1.4", " (SELECT 4 FROM t22 LIMIT 1) ", "integer", 4)
do_expr_test("e_expr-35.1.5", [[
(SELECT b FROM t22 UNION SELECT a+1 FROM t22)
]], "null", "")
@@ -3488,12 +3488,12 @@ do_expr_test("e_expr-35.1.6", [[
-- where a subquery returns more than one column.
--
data = {
- {1, "SELECT (SELECT * FROM t22 UNION SELECT a+1, b+1 FROM t22)"},
- {2, "SELECT (SELECT * FROM t22 UNION SELECT a+1, b+1 FROM t22 ORDER BY 1)"},
+ {1, "SELECT (SELECT * FROM t22 UNION SELECT a+1, b+1 FROM t22 LIMIT 1)"},
+ {2, "SELECT (SELECT * FROM t22 UNION SELECT a+1, b+1 FROM t22 ORDER BY 1 LIMIT 1)"},
{3, "SELECT (SELECT 1, 2)"},
{4, "SELECT (SELECT NULL, NULL, NULL)"},
- {5, "SELECT (SELECT * FROM t22)"},
- {6, "SELECT (SELECT * FROM (SELECT 1, 2, 3))"},
+ {5, "SELECT (SELECT * FROM t22 LIMIT 1)"},
+ {6, "SELECT (SELECT * FROM (SELECT 1, 2, 3) LIMIT 1)"},
}
local M = {1, "/sub--select returns [23] columns -- expected 1/"}
for _, val in ipairs(data) do
@@ -3524,11 +3524,11 @@ test:do_execsql_test(
})
data = {
- {2, "( SELECT x FROM t4 ORDER BY x ) ", "integer", 1},
- {3, "( SELECT x FROM t4 ORDER BY y ) ", "integer", 1},
- {4, "( SELECT x FROM t4 ORDER BY x DESC )", "integer", 3},
- {5, "( SELECT x FROM t4 ORDER BY y DESC )", "integer", 2},
- {6, "( SELECT y FROM t4 ORDER BY y DESC )", "text", "two"},
+ {2, "( SELECT x FROM t4 ORDER BY x LIMIT 1 ) ", "integer", 1},
+ {3, "( SELECT x FROM t4 ORDER BY y LIMIT 1 ) ", "integer", 1},
+ {4, "( SELECT x FROM t4 ORDER BY x DESC LIMIT 1)", "integer", 3},
+ {5, "( SELECT x FROM t4 ORDER BY y DESC LIMIT 1 )", "integer", 2},
+ {6, "( SELECT y FROM t4 ORDER BY y DESC LIMIT 1 )", "text", "two"},
{7, "( SELECT sum(x) FROM t4 ) ", "integer", 6},
{8, "( SELECT group_concat(y,'') FROM t4 )", "text", "onetwothree"},
{9, "( SELECT max(x) FROM t4 WHERE y LIKE '___')", "integer", 2 },
diff --git a/test/sql-tap/misc5.test.lua b/test/sql-tap/misc5.test.lua
index f25f8b2..09deeff 100755
--- a/test/sql-tap/misc5.test.lua
+++ b/test/sql-tap/misc5.test.lua
@@ -167,11 +167,7 @@ test:do_execsql_test(
WHERE songid IN (
SELECT songid
FROM songs
- WHERE LOWER(artist) = (
- -- This sub-query is indeterminate. Because there is no ORDER BY,
- -- it may return 'one', 'two' or 'three'. Because of this, the
- -- outermost parent query may correctly return any of 'one', 'two'
- -- or 'three' as well.
+ WHERE LOWER(artist) IN (
SELECT DISTINCT LOWER(artist)
FROM (
-- This sub-query returns the table:
@@ -186,14 +182,15 @@ test:do_execsql_test(
ORDER BY total DESC
LIMIT 10
)
- WHERE artist <> ''
- )
- )
+ WHERE artist <> ''
+ )
+ )
+ LIMIT 1
)
ORDER BY LOWER(artist) ASC;
]], {
-- <misc5-3.1>
- "two"
+ "one"
-- </misc5-3.1>
})
diff --git a/test/sql-tap/selectA.test.lua b/test/sql-tap/selectA.test.lua
index fc482e9..cf2ba64 100755
--- a/test/sql-tap/selectA.test.lua
+++ b/test/sql-tap/selectA.test.lua
@@ -2320,7 +2320,7 @@ test:do_execsql_test(
INTERSECT SELECT a,b,c FROM t3
EXCEPT SELECT c,b,a FROM t1
UNION SELECT a,b,c FROM t3
- ORDER BY y COLLATE "unicode_ci" DESC,x,z)))
+ ORDER BY y COLLATE "unicode_ci" DESC,x,z) LIMIT 1))
]], {
-- <selectA-3.97>
"MAD"
@@ -2340,7 +2340,7 @@ test:do_execsql_test(
INTERSECT SELECT a,b,c FROM t3
EXCEPT SELECT c,b,a FROM t1
UNION SELECT a,b,c FROM t3
- ORDER BY y COLLATE "unicode_ci" DESC,x,z)))
+ ORDER BY y COLLATE "unicode_ci" DESC,x,z LIMIT 1)))
UNION ALL
SELECT n || '+' FROM xyz WHERE length(n)<5
)
diff --git a/test/sql-tap/subquery.test.lua b/test/sql-tap/subquery.test.lua
index 06631c1..119f602 100755
--- a/test/sql-tap/subquery.test.lua
+++ b/test/sql-tap/subquery.test.lua
@@ -659,7 +659,7 @@ test:do_execsql_test(
test:do_execsql_test(
"subquery-4.1.1",
[[
- SELECT (SELECT a FROM t1);
+ SELECT (SELECT a FROM t1 LIMIT 1);
]], {
-- <subquery-4.1.1>
1
@@ -811,7 +811,7 @@ test:do_execsql_test(
INSERT INTO t9 VALUES(20000);
INSERT INTO t9 VALUES(30000);
- SELECT (SELECT c7+c8 FROM t7) FROM t8;
+ SELECT (SELECT c7+c8 FROM t7 LIMIT 1) FROM t8;
]], {
-- <subquery-7.1>
101, 201, 301
@@ -852,7 +852,7 @@ test:do_execsql_test(
test:do_execsql_test(
"subquery-7.6",
[[
- SELECT (SELECT (SELECT max(c7+c8+c9) FROM t9) FROM t8) FROM t7
+ SELECT (SELECT (SELECT max(c7+c8+c9) FROM t9 LIMIT 1) FROM t8 LIMIT 1) FROM t7
]], {
-- <subquery-7.6>
30101, 30102, 30103
@@ -862,7 +862,7 @@ test:do_execsql_test(
test:do_execsql_test(
"subquery-7.7",
[[
- SELECT (SELECT (SELECT c7+max(c8+c9) FROM t9) FROM t8) FROM t7
+ SELECT (SELECT (SELECT c7+max(c8+c9) FROM t9 LIMIT 1) FROM t8 LIMIT 1) FROM t7
]], {
-- <subquery-7.7>
30101, 30102, 30103
@@ -872,7 +872,7 @@ test:do_execsql_test(
test:do_execsql_test(
"subquery-7.8",
[[
- SELECT (SELECT (SELECT max(c7)+c8+c9 FROM t9) FROM t8) FROM t7
+ SELECT (SELECT (SELECT max(c7)+c8+c9 FROM t9 LIMIT 1) FROM t8 LIMIT 1) FROM t7
]], {
-- <subquery-7.8>
10103
@@ -882,7 +882,7 @@ test:do_execsql_test(
test:do_execsql_test(
"subquery-7.9",
[[
- SELECT (SELECT (SELECT c7+max(c8)+c9 FROM t9) FROM t8) FROM t7
+ SELECT (SELECT (SELECT c7+max(c8)+c9 FROM t9 LIMIT 1) FROM t8 LIMIT 1) FROM t7
]], {
-- <subquery-7.9>
10301, 10302, 10303
@@ -892,7 +892,7 @@ test:do_execsql_test(
test:do_execsql_test(
"subquery-7.10",
[[
- SELECT (SELECT (SELECT c7+c8+max(c9) FROM t9) FROM t8) FROM t7
+ SELECT (SELECT (SELECT c7+c8+max(c9) FROM t9 LIMIT 1) FROM t8 LIMIT 1) FROM t7
]], {
-- <subquery-7.10>
30101, 30102, 30103
diff --git a/test/sql-tap/subquery2.test.lua b/test/sql-tap/subquery2.test.lua
index 56daf7f..647a240 100755
--- a/test/sql-tap/subquery2.test.lua
+++ b/test/sql-tap/subquery2.test.lua
@@ -70,7 +70,7 @@ test:do_execsql_test(
"subquery2-1.11",
[[
SELECT a FROM t1
- WHERE +b=(SELECT x+1 FROM (SELECT DISTINCT f/(a*a) AS x FROM t3));
+ WHERE +b IN (SELECT x+1 FROM (SELECT DISTINCT f/(a*a) AS x FROM t3 LIMIT 1));
]], {
-- <subquery2-1.11>
1
@@ -81,7 +81,7 @@ test:do_execsql_test(
"subquery2-1.12",
[[
SELECT a FROM t1
- WHERE b=(SELECT x+1 FROM (SELECT DISTINCT f/(a*a) AS x FROM t3));
+ WHERE b IN (SELECT x+1 FROM (SELECT DISTINCT f/(a*a) AS x FROM t3 LIMIT 1));
]], {
-- <subquery2-1.12>
1
diff --git a/test/sql-tap/subselect.test.lua b/test/sql-tap/subselect.test.lua
index 809ec12..00faf9f 100755
--- a/test/sql-tap/subselect.test.lua
+++ b/test/sql-tap/subselect.test.lua
@@ -1,6 +1,6 @@
#!/usr/bin/env tarantool
test = require("sqltester")
-test:plan(26)
+test:plan(28)
--!./tcltestrunner.lua
-- 2001 September 15
@@ -162,7 +162,7 @@ test:do_test(
test:do_execsql_test(
"subselect-2.1",
[[
- SELECT (SELECT a FROM t1 ORDER BY a), (SELECT a FROM t1 ORDER BY a DESC)
+ SELECT (SELECT a FROM t1 ORDER BY a LIMIT 1), (SELECT a FROM t1 ORDER BY a DESC LIMIT 1)
]], {
-- <subselect-2.1>
1, 5
@@ -232,7 +232,7 @@ test:do_execsql_test(
test:do_execsql_test(
"subselect-3.4",
[[
- SELECT (SELECT x FROM t3 ORDER BY x);
+ SELECT (SELECT x FROM t3 ORDER BY x LIMIT 1);
]], {
-- <subselect-3.4>
1
@@ -242,7 +242,7 @@ test:do_execsql_test(
test:do_execsql_test(
"subselect-3.5",
[[
- SELECT (SELECT x FROM t3 ORDER BY x DESC);
+ SELECT (SELECT x FROM t3 ORDER BY x DESC LIMIT 1);
]], {
-- <subselect-3.5>
6
@@ -338,6 +338,30 @@ test:do_execsql_test(
-- </subselect-4.3>
})
+-- gh-2366 dissallow subselects returning multiple values
+test:do_catchsql_test(
+ "subselect-5.1",
+ [[
+ CREATE TABLE t5(a int primary key, b int);
+ INSERT INTO t5 VALUES(1,2);
+ INSERT INTO t5 VALUES(3,4);
+ INSERT INTO t5 VALUES(5,6);
+ INSERT INTO t5 VALUES(6,6);
+ SELECT (SELECT a FROM t5);
+ ]], {
+ -- <subselect-5.1>
+ 1, "SELECT subquery returned more than 1 row"
+ -- </subselect-5.1>
+})
+test:do_catchsql_test(
+ "subselect-5.2",
+ [[
+ SELECT b from t5 WHERE a = (SELECT a FROM t5 WHERE b=6);
+ ]], {
+ -- <subselect-5.2>
+ 1, "SELECT subquery returned more than 1 row"
+ -- </subselect-5.2>
+})
test:finish_test()
diff --git a/test/sql-tap/with1.test.lua b/test/sql-tap/with1.test.lua
index 528df67..6db8d13 100755
--- a/test/sql-tap/with1.test.lua
+++ b/test/sql-tap/with1.test.lua
@@ -609,7 +609,7 @@ test:do_execsql_test("8.2-soduko", [[
OR z.z = substr(s, ((ind-1)%9) + (lp-1)*9 + 1, 1)
OR z.z = substr(s, (((ind-1)/3) % 3) * 3
+ ((ind-1)/27) * 27 + lp
- + ((lp-1) / 3) * 6, 1)
+ + ((lp-1) / 3) * 6, 1) LIMIT 1
)
)
SELECT s FROM x WHERE ind=0;
--
2.7.4
More information about the Tarantool-patches
mailing list