[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