Tarantool development patches archive
 help / color / mirror / Atom feed
* [tarantool-patches] [PATCH v1 1/1] sql: disallow returning many rows from subselect
@ 2018-06-18 11:55 Kirill Shcherbatov
  2018-06-19 17:56 ` [tarantool-patches] " n.pettik
  2018-07-03  8:06 ` [tarantool-patches] Re: [PATCH v1 " Kirill Yukhin
  0 siblings, 2 replies; 15+ messages in thread
From: Kirill Shcherbatov @ 2018-06-18 11:55 UTC (permalink / raw)
  To: tarantool-patches; +Cc: v.shpilevoy, Kirill Shcherbatov

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [tarantool-patches] Re: [PATCH v1 1/1] sql: disallow returning many rows from subselect
  2018-06-18 11:55 [tarantool-patches] [PATCH v1 1/1] sql: disallow returning many rows from subselect Kirill Shcherbatov
@ 2018-06-19 17:56 ` n.pettik
  2018-06-27 15:30   ` [tarantool-patches] [PATCH v2 " Kirill Shcherbatov
  2018-06-27 15:40   ` Kirill Shcherbatov
  2018-07-03  8:06 ` [tarantool-patches] Re: [PATCH v1 " Kirill Yukhin
  1 sibling, 2 replies; 15+ messages in thread
From: n.pettik @ 2018-06-19 17:56 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Kirill Shcherbatov


> On 18 Jun 2018, at 14:55, Kirill Shcherbatov <kshcherbatov@tarantool.org> wrote:
> 
> 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.

This is not exactly correct. We disallow returning multiple rows only
when subselect is used in form of expression. In other words,
you are still able to use such subqueries with multiple rows/columns:

SELECT * FROM (SELECT * FROM t1);

It is also related to commit subject.

> To achieve this goal we have
> introduced some special

Lets throw away words like ’some’, ’special’ etc.:
they just contaminate commit message.

> 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:

From context it is not clear what you mean by iValue.

> this showld be done after simplifying epression tree(as this

Typo: should.

> restrict could become useless), but before generating related
> bytecode.
> 
> Resolves #2366
> —

Links to the branch and issue must be here,
not at the start of commit message.

> 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) {

I found that queries like

select a from t1 where a = (select a from t1 limit (select 1));

select a from t1 where a = (select a from t1 limit (2*1 - 1));

select a from t1 where a = (select a from t1 limit -1);

select a from t1 where a = (select a from t1 limit 0.5)

Also throw <- error: SELECT subquery returned more than 1 row>

In the third case error should be sort of:

<LIMIT must not be negative>

I guess SQLite allows negative values under LIMIT clause,
but it must be banned. I have opened issue for that:

https://github.com/tarantool/tarantool/issues/3467

In the fourth case we should raise <type mismatch> error.

As for the first two example, it seems to be correct:
SELECT 1; returns number literal 1.
2*1 - 1 == 1


> +				sql_expr_delete(pParse->db, pSel->pLimit, false);
> +				pSel->pLimit = sqlite3ExprAlloc(pParse->db, TK_INTEGER,

It doesn’t fit into 80 chars.

> +								&sqlite3IntTokens[1],
> +								0);
> +				pSel->selFlags |= SF_SingleRow;
> +			} else {
> +				/*
> +				 * Customer LIMIT 1 in subquery
> +				 * should be kept.

Do you need this empty ‘else’ clause only for comment?
You can put it above ‘if’ clause.
Lets use ‘user defined’ instead of ‘customer’.

> +				 */
> +			}
> 			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.
> +		 */

To be honest, I haven’t understood why we can’t make LIMIT be
LIMIT 2 right in sqlite3CodeSubselect()…

Examples above show that no simplifications under LIMIT clause occur:

select a from t1 where a = (select a from t1 limit (2*1 - 1));

If you mean that LIMIT clause may disappear, it also doesn’t seem to be true.
Consider this example:

 select a from t1 where a = (select a from t1 where 1 != 1 limit 3);

Obviously, subquery always returns 0 rows, but LIMIT is coded anyway:

SQL: [select a from t1 where a = (select a from t1 where 1 != 1 limit 3);]
VDBE Program Listing:
 101>    0 Init             0   25    0               00 Start at 25
 101>    1 LoadPtr          0    1    0 space<name=T1> 00 r[1] = space<name=T1>
 101>    2 OpenWrite        2 524288    1 k(1,B)        00 index id = 524288, space ptr = 1; sqlite_autoindex_T1_1
 101>    3 Explain          0    0    0 SCAN TABLE T1 00 
 101>    4 Rewind           2   24    2 0             00 
 101>    5 Column           2    1    2               00 r[2]=T1.A
 101>    6 Once             0   20    0               00 
 101>    7 Null             0    4    4               00 r[4..4]=NULL; Init subquery result
 101>    8 Integer          2    5    0               00 r[5]=2; LIMIT counter
 101>    9 Eq               6   17    6 (binary)      51 if r[6]==r[6] goto 17
 101>   10 LoadPtr          0    7    0 space<name=T1> 00 r[7] = space<name=T1>
 101>   11 OpenWrite        3 524288    7 k(1,B)        00 index id = 524288, space ptr = 7; sqlite_autoindex_T1_1
 101>   12 Explain          1    0    0 SCAN TABLE T1 00 
 101>   13 Rewind           3   17    8 0             00 
 101>   14 Column           3    1    4               00 r[4]=T1.A
 101>   15 DecrJumpZero     5   17    0               00 if (--r[5])==0 goto 17
 101>   16 Next             3   14    0               01 
 101>   17 Integer          0    8    0               00 r[8]=0
 101>   18 Ne               8   20    5               00 if r[5]!=r[8] goto 20
 101>   19 Halt            22    3    0 SELECT subquery returned more than 1 row 00 
 101>   20 Ne               4   23    2 (binary)      51 if r[2]!=r[4] goto 23
 101>   21 Column           2    1    9               00 r[9]=T1.A
 101>   22 ResultRow        9    1    0               00 output=r[9]
 101>   23 Next             2    5    0               01 
 101>   24 Halt             0    0    0               01 
 101>   25 Integer          1    6    0               00 r[6]=1
 101>   26 Goto             0    1    0               00 

> +		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. */

Prevent —> prevents from

> +	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”;

I guess, subquery term  assumes ’SELECT’ word in it, so lets drop it.
Also, I would mention, that it is 'subquery used as expression’.

> +		sqlite3VdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR,
> +				  ON_CONFLICT_ACTION_FAIL, 0,
> +				  error, P4_STATIC);

You may fit last two arguments in previous line, tho.

> +		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. */

Contain —> contains

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [tarantool-patches] [PATCH v2 1/1] sql: disallow returning many rows from subselect
  2018-06-19 17:56 ` [tarantool-patches] " n.pettik
@ 2018-06-27 15:30   ` Kirill Shcherbatov
  2018-06-27 15:40   ` Kirill Shcherbatov
  1 sibling, 0 replies; 15+ messages in thread
From: Kirill Shcherbatov @ 2018-06-27 15:30 UTC (permalink / raw)
  To: tarantool-patches; +Cc: n.pettik, Kirill Shcherbatov

To follow ANSI SQL standard we should dissallow returning
multiple rows from subselects after = and in braces ().
To achieve this goal we have introduced 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 extra bytecode to HALT execution on
reaching this restrict.

Resolves #2366
---
Branch: http://github.com/tarantool/tarantool/tree/kshch/gh-2366-whith-select-subquery
Issue: https://github.com/tarantool/tarantool/issues/2366

 src/box/sql/expr.c                              |  12 +-
 src/box/sql/select.c                            |  71 ++++-
 src/box/sql/sqliteInt.h                         |   4 +
 test/sql-tap/aggnested.test.lua                 |   2 +-
 test/sql-tap/e_expr.test.lua                    |  42 +--
 test/sql-tap/misc5.test.lua                     |  15 +-
 test/sql-tap/select4.test.lua                   |  22 +-
 test/sql-tap/select7.test.lua                   |   2 +-
 test/sql-tap/selectA.test.lua                   |  14 +-
 test/sql-tap/subquery.test.lua                  |  14 +-
 test/sql-tap/subquery2.test.lua                 |   4 +-
 test/sql-tap/subselect.test.lua                 |  34 ++-
 test/sql-tap/tkt1473.test.lua                   | 387 +-----------------------
 test/sql-tap/with1.test.lua                     |   2 +-
 test/sql/gh-2366-whith-select-subquery.result   |  56 ++++
 test/sql/gh-2366-whith-select-subquery.test.lua |  23 ++
 16 files changed, 237 insertions(+), 467 deletions(-)
 create mode 100644 test/sql/gh-2366-whith-select-subquery.result
 create mode 100644 test/sql/gh-2366-whith-select-subquery.test.lua

diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index 70e134f..23cee59 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -2874,10 +2874,14 @@ 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 =
+					sqlite3ExprAlloc(pParse->db, TK_INTEGER,
+							 &sqlite3IntTokens[1],
+							 0);
+				ExprSetProperty(pSel->pLimit, EP_System);
+			}
+			pSel->selFlags |= SF_SingleRow;
 			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 4e61ec1..1b42d50 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -2119,6 +2119,38 @@ computeLimitRegisters(Parse * pParse, Select * p, int iBreak)
 			sqlite3VdbeAddOp2(v, OP_IfNot, iLimit, iBreak);
 			VdbeCoverage(v);
 		}
+		if (p->selFlags & SF_SingleRow) {
+			if (ExprHasProperty(p->pLimit, EP_System)) {
+				/*
+				 * Indirect LIMIT 1 is allowed only for
+				 * requests returning only 1 row.
+				 * To test this, we change LIMIT 1 to
+				 * LIMIT 2 and will look up LIMIT 1 overflow
+				 * at the sqlite3Select end.
+				 */
+				sqlite3VdbeAddOp2(v, OP_Integer, 2, iLimit);
+			} else {
+				/*
+				 * User-defined complex limit for subquery
+				 * could be only 1 as resulting value.
+				 */
+				int r1 = sqlite3GetTempReg(pParse);
+				sqlite3VdbeAddOp2(v, OP_Integer, 1, r1);
+				int no_err = sqlite3VdbeMakeLabel(v);
+				sqlite3VdbeAddOp3(v, OP_Eq, iLimit, no_err, r1);
+				const char *error =
+					"SELECT subquery could be only limited "
+					"with 1";
+				sqlite3VdbeAddOp4(v, OP_Halt,
+						  SQL_TARANTOOL_ERROR,
+						  0, 0, error, P4_STATIC);
+				sqlite3VdbeResolveLabel(v, no_err);
+				sqlite3ReleaseTempReg(pParse, r1);
+
+				/* Runtime checks are no longer needed. */
+				p->selFlags &= ~SF_SingleRow;
+			}
+		}
 		if (p->pOffset) {
 			p->iOffset = iOffset = ++pParse->nMem;
 			pParse->nMem++;	/* Allocate an extra register for limit+offset */
@@ -5368,6 +5400,23 @@ explain_simple_count(struct Parse *parse_context, const char *table_name)
 	}
 }
 
+static void
+vdbe_code_raise_on_multiple_rows(struct Parse *parser, int limit_reg, int end_mark)
+{
+	assert(limit_reg != 0);
+	struct Vdbe *v = sqlite3GetVdbe(parser);
+	assert(v != NULL);
+
+	int r1 = sqlite3GetTempReg(parser);
+	sqlite3VdbeAddOp2(v, OP_Integer, 0, r1);
+	sqlite3VdbeAddOp3(v, OP_Ne, r1, end_mark, limit_reg);
+	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(parser, r1);
+}
+
 /*
  * Generate code for the SELECT statement given in the p argument.
  *
@@ -5404,6 +5453,14 @@ sqlite3Select(Parse * pParse,		/* The parser context */
 	int iRestoreSelectId = pParse->iSelectId;
 	pParse->iSelectId = pParse->iNextSelectId++;
 
+	/*
+	 * Keep system limit to raise error in sqlite3Select if subselect
+	 * would return multiple rows.
+	 */
+	struct Expr *system_limit = NULL;
+	if (p->pLimit != NULL && ExprHasProperty(p->pLimit, EP_System))
+		system_limit = p->pLimit;
+
 	db = pParse->db;
 	if (p == 0 || db->mallocFailed || pParse->nErr) {
 		return 1;
@@ -5504,6 +5561,12 @@ sqlite3Select(Parse * pParse,		/* The parser context */
 	if (p->pPrior) {
 		rc = multiSelect(pParse, p, pDest);
 		pParse->iSelectId = iRestoreSelectId;
+
+		int end = sqlite3VdbeMakeLabel(v);
+		if (p->selFlags & SF_SingleRow && p->iLimit != 0)
+			vdbe_code_raise_on_multiple_rows(pParse, p->iLimit, end);
+		sqlite3VdbeResolveLabel(v, end);
+
 #ifdef SELECTTRACE_ENABLED
 		SELECTTRACE(1, pParse, p, ("end compound-select processing\n"));
 		pParse->nSelectIndent--;
@@ -5740,6 +5803,8 @@ sqlite3Select(Parse * pParse,		/* The parser context */
 	if ((p->selFlags & SF_FixedLimit) == 0) {
 		p->nSelectRow = 320;	/* 4 billion rows */
 	}
+	if (system_limit != NULL)
+		p->pLimit = system_limit;
 	computeLimitRegisters(pParse, p, iEnd);
 	if (p->iLimit == 0 && sSort.addrSortIndex >= 0) {
 		sqlite3VdbeChangeOpcode(v, sSort.addrSortIndex, OP_SorterOpen);
@@ -6296,8 +6361,10 @@ 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. */
+	if (p->selFlags & SF_SingleRow && p->iLimit != 0)
+		vdbe_code_raise_on_multiple_rows(pParse, p->iLimit, iEnd);
+	/* 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 5f7a0f1..093aabb 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -2370,6 +2370,8 @@ struct Expr {
 #define EP_Subquery  0x200000	/* Tree contains a TK_SELECT operator */
 #define EP_Alias     0x400000	/* Is an alias for a result set column */
 #define EP_Leaf      0x800000	/* Expr.pLeft, .pRight, .u.pSelect all NULL */
+/** Expression is system-defined. */
+#define EP_System    0x1000000
 
 /*
  * Combinations of two or more EP_* flags
@@ -2718,6 +2720,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..8525ce2 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,12 +3474,12 @@ 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)
+  (SELECT b FROM t22 UNION SELECT a+1 FROM t22 LIMIT 1)
 ]], "null", "")
 do_expr_test("e_expr-35.1.6", [[ 
-  (SELECT a FROM t22 UNION SELECT COALESCE(b, 55) FROM t22 ORDER BY 1)
+  (SELECT a FROM t22 UNION SELECT COALESCE(b, 55) FROM t22 ORDER BY 1 LIMIT 1)
 ]], "integer", 4)
 -- EVIDENCE-OF: R-46899-53765 A SELECT used as a scalar quantity must
 -- return a result set with a single column.
@@ -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/select4.test.lua b/test/sql-tap/select4.test.lua
index 5f91b13..5b49d8b 100755
--- a/test/sql-tap/select4.test.lua
+++ b/test/sql-tap/select4.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(103)
+test:plan(101)
 
 --!./tcltestrunner.lua
 -- 2001 September 15
@@ -1379,26 +1379,6 @@ test:do_execsql_test(
     })
 
 test:do_execsql_test(
-    "select4-14.10",
-    [[
-        SELECT (VALUES(1),(2),(3),(4))
-    ]], {
-        -- <select4-14.10>
-        1
-        -- </select4-14.10>
-    })
-
-test:do_execsql_test(
-    "select4-14.11",
-    [[
-        SELECT (SELECT 1 UNION ALL SELECT 2 UNION ALL SELECT 3 UNION ALL SELECT 4)
-    ]], {
-        -- <select4-14.11>
-        1
-        -- </select4-14.11>
-    })
-
-test:do_execsql_test(
     "select4-14.12",
     [[
         VALUES(1) UNION VALUES(2);
diff --git a/test/sql-tap/select7.test.lua b/test/sql-tap/select7.test.lua
index 7fc0a3c..10e13e2 100755
--- a/test/sql-tap/select7.test.lua
+++ b/test/sql-tap/select7.test.lua
@@ -112,7 +112,7 @@ test:do_execsql_test(
         SELECT P.pk from PHOTO P WHERE NOT EXISTS (
              SELECT T2.pk from TAG T2 WHERE T2.fk = P.pk
              EXCEPT
-             SELECT T3.pk from TAG T3 WHERE T3.fk = P.pk AND T3.name LIKE '%foo%'
+             SELECT T3.pk from TAG T3 WHERE T3.fk = P.pk AND T3.name LIKE '%foo%' LIMIT 1
         );
     ]], {
         -- <select7-4.2>
diff --git a/test/sql-tap/selectA.test.lua b/test/sql-tap/selectA.test.lua
index fc482e9..367a3f1 100755
--- a/test/sql-tap/selectA.test.lua
+++ b/test/sql-tap/selectA.test.lua
@@ -1145,7 +1145,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "selectA-2.93",
     [[
-        SELECT upper((SELECT c FROM t1 UNION SELECT z FROM t2 ORDER BY 1));
+        SELECT upper((SELECT c FROM t1 UNION SELECT z FROM t2 ORDER BY 1 LIMIT 1));
     ]], {
         -- <selectA-2.93>
         "A"
@@ -1155,7 +1155,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "selectA-2.94",
     [[
-        SELECT lower((SELECT c FROM t1 UNION ALL SELECT z FROM t2 ORDER BY 1));
+        SELECT lower((SELECT c FROM t1 UNION ALL SELECT z FROM t2 ORDER BY 1 LIMIT 1));
     ]], {
         -- <selectA-2.94>
         "a"
@@ -1165,7 +1165,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "selectA-2.95",
     [[
-        SELECT lower((SELECT c FROM t1 INTERSECT SELECT z FROM t2 ORDER BY 1));
+        SELECT lower((SELECT c FROM t1 INTERSECT SELECT z FROM t2 ORDER BY 1 LIMIT 1));
     ]], {
         -- <selectA-2.95>
         ""
@@ -1175,7 +1175,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "selectA-2.96",
     [[
-        SELECT lower((SELECT z FROM t2 EXCEPT SELECT c FROM t1 ORDER BY 1));
+        SELECT lower((SELECT z FROM t2 EXCEPT SELECT c FROM t1 ORDER BY 1 LIMIT 1));
     ]], {
         -- <selectA-2.96>
         "m"
@@ -2302,7 +2302,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "selectA-3.96",
     [[
-        SELECT lower((SELECT z FROM t2 EXCEPT SELECT c FROM t1 ORDER BY 1));
+        SELECT lower((SELECT z FROM t2 EXCEPT SELECT c FROM t1 ORDER BY 1 LIMIT 1));
     ]], {
         -- <selectA-3.96>
         "m"
@@ -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..3562c9a 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
@@ -99,7 +99,7 @@ test:do_execsql_test(
     "subselect-1.3e",
     [[
         SELECT b FROM t1
-         WHERE a = (SELECT a FROM t1 UNION SELECT b FROM t1 ORDER BY 1);
+         WHERE a = (SELECT a FROM t1 UNION SELECT b FROM t1 ORDER BY 1 LIMIT 1);
     ]], {
         -- <subselect-1.3e>
         2
@@ -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/tkt1473.test.lua b/test/sql-tap/tkt1473.test.lua
index c9b53b1..f042ca3 100755
--- a/test/sql-tap/tkt1473.test.lua
+++ b/test/sql-tap/tkt1473.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(57)
+test:plan(39)
 
 --!./tcltestrunner.lua
 -- 2005 September 19
@@ -118,17 +118,6 @@ test:do_execsql_test(
 -- Everything from this point on depends on sub-queries. So skip it
 -- if sub-queries are not available.
 
-
-test:do_execsql_test(
-    "tkt1473-2.2",
-    [[
-        SELECT (SELECT 1 FROM t1 WHERE a=1 UNION ALL SELECT 2 FROM t1 WHERE b=0)
-    ]], {
-        -- <tkt1473-2.2>
-        1
-        -- </tkt1473-2.2>
-    })
-
 test:do_execsql_test(
     "tkt1473-2.3",
     [[
@@ -140,36 +129,6 @@ test:do_execsql_test(
     })
 
 test:do_execsql_test(
-    "tkt1473-2.4",
-    [[
-        SELECT (SELECT 1 FROM t1 WHERE a=1 UNION ALL SELECT 2 FROM t1 WHERE b=4)
-    ]], {
-        -- <tkt1473-2.4>
-        1
-        -- </tkt1473-2.4>
-    })
-
-test:do_execsql_test(
-    "tkt1473-2.5",
-    [[
-        SELECT (SELECT 1 FROM t1 WHERE a=1 UNION SELECT 2 FROM t1 WHERE b=4)
-    ]], {
-        -- <tkt1473-2.5>
-        1
-        -- </tkt1473-2.5>
-    })
-
-test:do_execsql_test(
-    "tkt1473-2.6",
-    [[
-        SELECT (SELECT 1 FROM t1 WHERE a=0 UNION ALL SELECT 2 FROM t1 WHERE b=4)
-    ]], {
-        -- <tkt1473-2.6>
-        2
-        -- </tkt1473-2.6>
-    })
-
-test:do_execsql_test(
     "tkt1473-2.7",
     [[
         SELECT (SELECT 1 FROM t1 WHERE a=0 UNION SELECT 2 FROM t1 WHERE b=4)
@@ -200,17 +159,6 @@ test:do_execsql_test(
     })
 
 test:do_execsql_test(
-    "tkt1473-3.2",
-    [[
-        SELECT EXISTS
-          (SELECT 1 FROM t1 WHERE a=1 UNION ALL SELECT 2 FROM t1 WHERE b=0)
-    ]], {
-        -- <tkt1473-3.2>
-        1
-        -- </tkt1473-3.2>
-    })
-
-test:do_execsql_test(
     "tkt1473-3.3",
     [[
         SELECT EXISTS
@@ -222,39 +170,6 @@ test:do_execsql_test(
     })
 
 test:do_execsql_test(
-    "tkt1473-3.4",
-    [[
-        SELECT EXISTS
-          (SELECT 1 FROM t1 WHERE a=1 UNION ALL SELECT 2 FROM t1 WHERE b=4)
-    ]], {
-        -- <tkt1473-3.4>
-        1
-        -- </tkt1473-3.4>
-    })
-
-test:do_execsql_test(
-    "tkt1473-3.5",
-    [[
-        SELECT EXISTS
-          (SELECT 1 FROM t1 WHERE a=1 UNION SELECT 2 FROM t1 WHERE b=4)
-    ]], {
-        -- <tkt1473-3.5>
-        1
-        -- </tkt1473-3.5>
-    })
-
-test:do_execsql_test(
-    "tkt1473-3.6",
-    [[
-        SELECT EXISTS
-          (SELECT 1 FROM t1 WHERE a=0 UNION ALL SELECT 2 FROM t1 WHERE b=4)
-    ]], {
-        -- <tkt1473-3.6>
-        1
-        -- </tkt1473-3.6>
-    })
-
-test:do_execsql_test(
     "tkt1473-3.7",
     [[
         SELECT EXISTS
@@ -334,126 +249,6 @@ test:do_execsql_test(
     })
 
 test:do_execsql_test(
-    "tkt1473-4.3",
-    [[
-        SELECT (
-          SELECT 1 FROM t2 WHERE x=0
-          UNION ALL
-          SELECT 2 FROM t2 WHERE x=1
-          UNION ALL
-          SELECT 3 FROM t2 WHERE x=2
-          UNION ALL
-          SELECT 4 FROM t2 WHERE x=3
-          UNION ALL
-          SELECT 5 FROM t2 WHERE x=4
-          UNION ALL
-          SELECT 6 FROM t2 WHERE y=0
-          UNION ALL
-          SELECT 7 FROM t2 WHERE y=1
-          UNION ALL
-          SELECT 8 FROM t2 WHERE y=2
-          UNION ALL
-          SELECT 9 FROM t2 WHERE y=3
-          UNION ALL
-          SELECT 10 FROM t2 WHERE y=4
-        )
-    ]], {
-        -- <tkt1473-4.3>
-        2
-        -- </tkt1473-4.3>
-    })
-
-test:do_execsql_test(
-    "tkt1473-4.4",
-    [[
-        SELECT (
-          SELECT 1 FROM t2 WHERE x=0
-          UNION ALL
-          SELECT 2 FROM t2 WHERE x=-1
-          UNION ALL
-          SELECT 3 FROM t2 WHERE x=2
-          UNION ALL
-          SELECT 4 FROM t2 WHERE x=3
-          UNION ALL
-          SELECT 5 FROM t2 WHERE x=4
-          UNION ALL
-          SELECT 6 FROM t2 WHERE y=0
-          UNION ALL
-          SELECT 7 FROM t2 WHERE y=1
-          UNION ALL
-          SELECT 8 FROM t2 WHERE y=2
-          UNION ALL
-          SELECT 9 FROM t2 WHERE y=3
-          UNION ALL
-          SELECT 10 FROM t2 WHERE y=4
-        )
-    ]], {
-        -- <tkt1473-4.4>
-        4
-        -- </tkt1473-4.4>
-    })
-
-test:do_execsql_test(
-    "tkt1473-4.5",
-    [[
-        SELECT (
-          SELECT 1 FROM t2 WHERE x=0
-          UNION ALL
-          SELECT 2 FROM t2 WHERE x=-1
-          UNION ALL
-          SELECT 3 FROM t2 WHERE x=2
-          UNION ALL
-          SELECT 4 FROM t2 WHERE x=-1
-          UNION ALL
-          SELECT 5 FROM t2 WHERE x=4
-          UNION ALL
-          SELECT 6 FROM t2 WHERE y=0
-          UNION ALL
-          SELECT 7 FROM t2 WHERE y=1
-          UNION ALL
-          SELECT 8 FROM t2 WHERE y=2
-          UNION ALL
-          SELECT 9 FROM t2 WHERE y=3
-          UNION ALL
-          SELECT 10 FROM t2 WHERE y=-4
-        )
-    ]], {
-        -- <tkt1473-4.5>
-        8
-        -- </tkt1473-4.5>
-    })
-
-test:do_execsql_test(
-    "tkt1473-4.6",
-    [[
-        SELECT (
-          SELECT 1 FROM t2 WHERE x=0
-          UNION ALL
-          SELECT 2 FROM t2 WHERE x=-1
-          UNION ALL
-          SELECT 3 FROM t2 WHERE x=2
-          UNION ALL
-          SELECT 4 FROM t2 WHERE x=-2
-          UNION ALL
-          SELECT 5 FROM t2 WHERE x=4
-          UNION ALL
-          SELECT 6 FROM t2 WHERE y=0
-          UNION ALL
-          SELECT 7 FROM t2 WHERE y=1
-          UNION ALL
-          SELECT 8 FROM t2 WHERE y=-3
-          UNION ALL
-          SELECT 9 FROM t2 WHERE y=3
-          UNION ALL
-          SELECT 10 FROM t2 WHERE y=4
-        )
-    ]], {
-        -- <tkt1473-4.6>
-        10
-        -- </tkt1473-4.6>
-    })
-
-test:do_execsql_test(
     "tkt1473-4.7",
     [[
         SELECT (
@@ -484,126 +279,6 @@ test:do_execsql_test(
     })
 
 test:do_execsql_test(
-    "tkt1473-5.3",
-    [[
-        SELECT EXISTS (
-          SELECT 1 FROM t2 WHERE x=0
-          UNION ALL
-          SELECT 2 FROM t2 WHERE x=1
-          UNION ALL
-          SELECT 3 FROM t2 WHERE x=2
-          UNION ALL
-          SELECT 4 FROM t2 WHERE x=3
-          UNION ALL
-          SELECT 5 FROM t2 WHERE x=4
-          UNION ALL
-          SELECT 6 FROM t2 WHERE y=0
-          UNION ALL
-          SELECT 7 FROM t2 WHERE y=1
-          UNION ALL
-          SELECT 8 FROM t2 WHERE y=2
-          UNION ALL
-          SELECT 9 FROM t2 WHERE y=3
-          UNION ALL
-          SELECT 10 FROM t2 WHERE y=4
-        )
-    ]], {
-        -- <tkt1473-5.3>
-        1
-        -- </tkt1473-5.3>
-    })
-
-test:do_execsql_test(
-    "tkt1473-5.4",
-    [[
-        SELECT EXISTS (
-          SELECT 1 FROM t2 WHERE x=0
-          UNION ALL
-          SELECT 2 FROM t2 WHERE x=-1
-          UNION ALL
-          SELECT 3 FROM t2 WHERE x=2
-          UNION ALL
-          SELECT 4 FROM t2 WHERE x=3
-          UNION ALL
-          SELECT 5 FROM t2 WHERE x=4
-          UNION ALL
-          SELECT 6 FROM t2 WHERE y=0
-          UNION ALL
-          SELECT 7 FROM t2 WHERE y=1
-          UNION ALL
-          SELECT 8 FROM t2 WHERE y=2
-          UNION ALL
-          SELECT 9 FROM t2 WHERE y=3
-          UNION ALL
-          SELECT 10 FROM t2 WHERE y=4
-        )
-    ]], {
-        -- <tkt1473-5.4>
-        1
-        -- </tkt1473-5.4>
-    })
-
-test:do_execsql_test(
-    "tkt1473-5.5",
-    [[
-        SELECT EXISTS (
-          SELECT 1 FROM t2 WHERE x=0
-          UNION ALL
-          SELECT 2 FROM t2 WHERE x=-1
-          UNION ALL
-          SELECT 3 FROM t2 WHERE x=2
-          UNION ALL
-          SELECT 4 FROM t2 WHERE x=-1
-          UNION ALL
-          SELECT 5 FROM t2 WHERE x=4
-          UNION ALL
-          SELECT 6 FROM t2 WHERE y=0
-          UNION ALL
-          SELECT 7 FROM t2 WHERE y=1
-          UNION ALL
-          SELECT 8 FROM t2 WHERE y=2
-          UNION ALL
-          SELECT 9 FROM t2 WHERE y=3
-          UNION ALL
-          SELECT 10 FROM t2 WHERE y=-4
-        )
-    ]], {
-        -- <tkt1473-5.5>
-        1
-        -- </tkt1473-5.5>
-    })
-
-test:do_execsql_test(
-    "tkt1473-5.6",
-    [[
-        SELECT EXISTS (
-          SELECT 1 FROM t2 WHERE x=0
-          UNION ALL
-          SELECT 2 FROM t2 WHERE x=-1
-          UNION ALL
-          SELECT 3 FROM t2 WHERE x=2
-          UNION ALL
-          SELECT 4 FROM t2 WHERE x=-2
-          UNION ALL
-          SELECT 5 FROM t2 WHERE x=4
-          UNION ALL
-          SELECT 6 FROM t2 WHERE y=0
-          UNION ALL
-          SELECT 7 FROM t2 WHERE y=1
-          UNION ALL
-          SELECT 8 FROM t2 WHERE y=-3
-          UNION ALL
-          SELECT 9 FROM t2 WHERE y=3
-          UNION ALL
-          SELECT 10 FROM t2 WHERE y=4
-        )
-    ]], {
-        -- <tkt1473-5.6>
-        1
-        -- </tkt1473-5.6>
-    })
-
-test:do_execsql_test(
     "tkt1473-5.7",
     [[
         SELECT EXISTS (
@@ -634,66 +309,6 @@ test:do_execsql_test(
     })
 
 test:do_execsql_test(
-    "tkt1473-6.3",
-    [[
-        SELECT EXISTS (
-          SELECT 1 FROM t2 WHERE x=0
-          UNION
-          SELECT 2 FROM t2 WHERE x=1
-          UNION
-          SELECT 3 FROM t2 WHERE x=2
-          UNION
-          SELECT 4 FROM t2 WHERE x=3
-          UNION
-          SELECT 5 FROM t2 WHERE x=4
-          UNION
-          SELECT 6 FROM t2 WHERE y=0
-          UNION
-          SELECT 7 FROM t2 WHERE y=1
-          UNION
-          SELECT 8 FROM t2 WHERE y=2
-          UNION
-          SELECT 9 FROM t2 WHERE y=3
-          UNION
-          SELECT 10 FROM t2 WHERE y=4
-        )
-    ]], {
-        -- <tkt1473-6.3>
-        1
-        -- </tkt1473-6.3>
-    })
-
-test:do_execsql_test(
-    "tkt1473-6.4",
-    [[
-        SELECT EXISTS (
-          SELECT 1 FROM t2 WHERE x=0
-          UNION
-          SELECT 2 FROM t2 WHERE x=-1
-          UNION
-          SELECT 3 FROM t2 WHERE x=2
-          UNION
-          SELECT 4 FROM t2 WHERE x=3
-          UNION
-          SELECT 5 FROM t2 WHERE x=4
-          UNION
-          SELECT 6 FROM t2 WHERE y=0
-          UNION
-          SELECT 7 FROM t2 WHERE y=1
-          UNION
-          SELECT 8 FROM t2 WHERE y=2
-          UNION
-          SELECT 9 FROM t2 WHERE y=3
-          UNION
-          SELECT 10 FROM t2 WHERE y=4
-        )
-    ]], {
-        -- <tkt1473-6.4>
-        1
-        -- </tkt1473-6.4>
-    })
-
-test:do_execsql_test(
     "tkt1473-6.5",
     [[
         SELECT EXISTS (
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;
diff --git a/test/sql/gh-2366-whith-select-subquery.result b/test/sql/gh-2366-whith-select-subquery.result
new file mode 100644
index 0000000..e6edf2b
--- /dev/null
+++ b/test/sql/gh-2366-whith-select-subquery.result
@@ -0,0 +1,56 @@
+env = require('test_run')
+---
+...
+test_run = env.new()
+---
+...
+--
+-- gh-2366: SQL subquery with =
+--
+box.sql.execute("CREATE TABLE t1(a int primary key, b int);")
+---
+...
+box.sql.execute("INSERT INTO t1 VALUES(1,2);")
+---
+...
+box.sql.execute("INSERT INTO t1 VALUES(3,4);")
+---
+...
+box.sql.execute("INSERT INTO t1 VALUES(5,6);")
+---
+...
+box.sql.execute("INSERT INTO t1 VALUES(7,6);")
+---
+...
+box.sql.execute("select * from t1;")
+---
+- - [1, 2]
+  - [3, 4]
+  - [5, 6]
+  - [7, 6]
+...
+box.sql.execute("SELECT a FROM t1 WHERE b=6 LIMIT (select b from t1 where a =1);")
+---
+- - [5]
+  - [7]
+...
+box.sql.execute("SELECT b from t1 where a = (SELECT a FROM t1 WHERE b=6);")
+---
+- error: SELECT subquery returned more than 1 row
+...
+box.sql.execute("SELECT b from t1 where a = (SELECT a FROM t1 WHERE b=6 LIMIT (select b-1 from t1 where a =1));")
+---
+- - [6]
+...
+box.sql.execute("SELECT b from t1 where a = (SELECT a FROM t1 WHERE b=6 LIMIT (select b from t1 where a =1));")
+---
+- error: SELECT subquery could be only limited with 1
+...
+box.sql.execute("SELECT (VALUES(1),(2),(3),(4));")
+---
+- error: SELECT subquery returned more than 1 row
+...
+-- clean-up
+box.sql.execute("DROP TABLE t1;")
+---
+...
diff --git a/test/sql/gh-2366-whith-select-subquery.test.lua b/test/sql/gh-2366-whith-select-subquery.test.lua
new file mode 100644
index 0000000..39116df
--- /dev/null
+++ b/test/sql/gh-2366-whith-select-subquery.test.lua
@@ -0,0 +1,23 @@
+env = require('test_run')
+test_run = env.new()
+
+--
+-- gh-2366: SQL subquery with =
+--
+
+box.sql.execute("CREATE TABLE t1(a int primary key, b int);")
+box.sql.execute("INSERT INTO t1 VALUES(1,2);")
+box.sql.execute("INSERT INTO t1 VALUES(3,4);")
+box.sql.execute("INSERT INTO t1 VALUES(5,6);")
+box.sql.execute("INSERT INTO t1 VALUES(7,6);")
+
+box.sql.execute("select * from t1;")
+box.sql.execute("SELECT a FROM t1 WHERE b=6 LIMIT (select b from t1 where a =1);")
+box.sql.execute("SELECT b from t1 where a = (SELECT a FROM t1 WHERE b=6);")
+box.sql.execute("SELECT b from t1 where a = (SELECT a FROM t1 WHERE b=6 LIMIT (select b-1 from t1 where a =1));")
+box.sql.execute("SELECT b from t1 where a = (SELECT a FROM t1 WHERE b=6 LIMIT (select b from t1 where a =1));")
+
+box.sql.execute("SELECT (VALUES(1),(2),(3),(4));")
+
+-- clean-up
+box.sql.execute("DROP TABLE t1;")
\ No newline at end of file
-- 
2.7.4

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [tarantool-patches] [PATCH v2 1/1] sql: disallow returning many rows from subselect
  2018-06-19 17:56 ` [tarantool-patches] " n.pettik
  2018-06-27 15:30   ` [tarantool-patches] [PATCH v2 " Kirill Shcherbatov
@ 2018-06-27 15:40   ` Kirill Shcherbatov
  2018-06-28 13:04     ` [tarantool-patches] " n.pettik
  1 sibling, 1 reply; 15+ messages in thread
From: Kirill Shcherbatov @ 2018-06-27 15:40 UTC (permalink / raw)
  To: tarantool-patches; +Cc: korablev, Kirill Shcherbatov

To follow ANSI SQL standard we should dissallow returning
multiple rows from subselects after = and in braces ().
To achieve this goal we have introduced 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 extra bytecode to HALT execution on
reaching this restrict.

Resolves #2366
---
Branch: http://github.com/tarantool/tarantool/tree/kshch/gh-2366-whith-select-subquery
Issue: https://github.com/tarantool/tarantool/issues/2366

 src/box/sql/expr.c                              |  12 +-
 src/box/sql/select.c                            |  61 +++-
 src/box/sql/sqliteInt.h                         |   4 +
 test/sql-tap/aggnested.test.lua                 |   2 +-
 test/sql-tap/e_expr.test.lua                    |  42 +--
 test/sql-tap/misc5.test.lua                     |  15 +-
 test/sql-tap/select4.test.lua                   |  22 +-
 test/sql-tap/select7.test.lua                   |   2 +-
 test/sql-tap/selectA.test.lua                   |  14 +-
 test/sql-tap/subquery.test.lua                  |  14 +-
 test/sql-tap/subquery2.test.lua                 |   4 +-
 test/sql-tap/subselect.test.lua                 |  34 ++-
 test/sql-tap/tkt1473.test.lua                   | 387 +-----------------------
 test/sql-tap/with1.test.lua                     |   2 +-
 test/sql/gh-2366-whith-select-subquery.result   |  56 ++++
 test/sql/gh-2366-whith-select-subquery.test.lua |  23 ++
 16 files changed, 227 insertions(+), 467 deletions(-)
 create mode 100644 test/sql/gh-2366-whith-select-subquery.result
 create mode 100644 test/sql/gh-2366-whith-select-subquery.test.lua

diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index 70e134f..23cee59 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -2874,10 +2874,14 @@ 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 =
+					sqlite3ExprAlloc(pParse->db, TK_INTEGER,
+							 &sqlite3IntTokens[1],
+							 0);
+				ExprSetProperty(pSel->pLimit, EP_System);
+			}
+			pSel->selFlags |= SF_SingleRow;
 			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 4e61ec1..e904c2a 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -2119,6 +2119,38 @@ computeLimitRegisters(Parse * pParse, Select * p, int iBreak)
 			sqlite3VdbeAddOp2(v, OP_IfNot, iLimit, iBreak);
 			VdbeCoverage(v);
 		}
+		if (p->selFlags & SF_SingleRow) {
+			if (ExprHasProperty(p->pLimit, EP_System)) {
+				/*
+				 * Indirect LIMIT 1 is allowed only for
+				 * requests returning only 1 row.
+				 * To test this, we change LIMIT 1 to
+				 * LIMIT 2 and will look up LIMIT 1 overflow
+				 * at the sqlite3Select end.
+				 */
+				sqlite3VdbeAddOp2(v, OP_Integer, 2, iLimit);
+			} else {
+				/*
+				 * User-defined complex limit for subquery
+				 * could be only 1 as resulting value.
+				 */
+				int r1 = sqlite3GetTempReg(pParse);
+				sqlite3VdbeAddOp2(v, OP_Integer, 1, r1);
+				int no_err = sqlite3VdbeMakeLabel(v);
+				sqlite3VdbeAddOp3(v, OP_Eq, iLimit, no_err, r1);
+				const char *error =
+					"SELECT subquery could be only limited "
+					"with 1";
+				sqlite3VdbeAddOp4(v, OP_Halt,
+						  SQL_TARANTOOL_ERROR,
+						  0, 0, error, P4_STATIC);
+				sqlite3VdbeResolveLabel(v, no_err);
+				sqlite3ReleaseTempReg(pParse, r1);
+
+				/* Runtime checks are no longer needed. */
+				p->selFlags &= ~SF_SingleRow;
+			}
+		}
 		if (p->pOffset) {
 			p->iOffset = iOffset = ++pParse->nMem;
 			pParse->nMem++;	/* Allocate an extra register for limit+offset */
@@ -5368,6 +5400,23 @@ explain_simple_count(struct Parse *parse_context, const char *table_name)
 	}
 }
 
+static void
+vdbe_code_raise_on_multiple_rows(struct Parse *parser, int limit_reg, int end_mark)
+{
+	assert(limit_reg != 0);
+	struct Vdbe *v = sqlite3GetVdbe(parser);
+	assert(v != NULL);
+
+	int r1 = sqlite3GetTempReg(parser);
+	sqlite3VdbeAddOp2(v, OP_Integer, 0, r1);
+	sqlite3VdbeAddOp3(v, OP_Ne, r1, end_mark, limit_reg);
+	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(parser, r1);
+}
+
 /*
  * Generate code for the SELECT statement given in the p argument.
  *
@@ -5504,6 +5553,12 @@ sqlite3Select(Parse * pParse,		/* The parser context */
 	if (p->pPrior) {
 		rc = multiSelect(pParse, p, pDest);
 		pParse->iSelectId = iRestoreSelectId;
+
+		int end = sqlite3VdbeMakeLabel(v);
+		if (p->selFlags & SF_SingleRow && p->iLimit != 0)
+			vdbe_code_raise_on_multiple_rows(pParse, p->iLimit, end);
+		sqlite3VdbeResolveLabel(v, end);
+
 #ifdef SELECTTRACE_ENABLED
 		SELECTTRACE(1, pParse, p, ("end compound-select processing\n"));
 		pParse->nSelectIndent--;
@@ -6296,8 +6351,10 @@ 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. */
+	if (p->selFlags & SF_SingleRow && p->iLimit != 0)
+		vdbe_code_raise_on_multiple_rows(pParse, p->iLimit, iEnd);
+	/* 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 5f7a0f1..093aabb 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -2370,6 +2370,8 @@ struct Expr {
 #define EP_Subquery  0x200000	/* Tree contains a TK_SELECT operator */
 #define EP_Alias     0x400000	/* Is an alias for a result set column */
 #define EP_Leaf      0x800000	/* Expr.pLeft, .pRight, .u.pSelect all NULL */
+/** Expression is system-defined. */
+#define EP_System    0x1000000
 
 /*
  * Combinations of two or more EP_* flags
@@ -2718,6 +2720,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..8525ce2 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,12 +3474,12 @@ 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)
+  (SELECT b FROM t22 UNION SELECT a+1 FROM t22 LIMIT 1)
 ]], "null", "")
 do_expr_test("e_expr-35.1.6", [[ 
-  (SELECT a FROM t22 UNION SELECT COALESCE(b, 55) FROM t22 ORDER BY 1)
+  (SELECT a FROM t22 UNION SELECT COALESCE(b, 55) FROM t22 ORDER BY 1 LIMIT 1)
 ]], "integer", 4)
 -- EVIDENCE-OF: R-46899-53765 A SELECT used as a scalar quantity must
 -- return a result set with a single column.
@@ -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/select4.test.lua b/test/sql-tap/select4.test.lua
index 5f91b13..5b49d8b 100755
--- a/test/sql-tap/select4.test.lua
+++ b/test/sql-tap/select4.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(103)
+test:plan(101)
 
 --!./tcltestrunner.lua
 -- 2001 September 15
@@ -1379,26 +1379,6 @@ test:do_execsql_test(
     })
 
 test:do_execsql_test(
-    "select4-14.10",
-    [[
-        SELECT (VALUES(1),(2),(3),(4))
-    ]], {
-        -- <select4-14.10>
-        1
-        -- </select4-14.10>
-    })
-
-test:do_execsql_test(
-    "select4-14.11",
-    [[
-        SELECT (SELECT 1 UNION ALL SELECT 2 UNION ALL SELECT 3 UNION ALL SELECT 4)
-    ]], {
-        -- <select4-14.11>
-        1
-        -- </select4-14.11>
-    })
-
-test:do_execsql_test(
     "select4-14.12",
     [[
         VALUES(1) UNION VALUES(2);
diff --git a/test/sql-tap/select7.test.lua b/test/sql-tap/select7.test.lua
index 7fc0a3c..10e13e2 100755
--- a/test/sql-tap/select7.test.lua
+++ b/test/sql-tap/select7.test.lua
@@ -112,7 +112,7 @@ test:do_execsql_test(
         SELECT P.pk from PHOTO P WHERE NOT EXISTS (
              SELECT T2.pk from TAG T2 WHERE T2.fk = P.pk
              EXCEPT
-             SELECT T3.pk from TAG T3 WHERE T3.fk = P.pk AND T3.name LIKE '%foo%'
+             SELECT T3.pk from TAG T3 WHERE T3.fk = P.pk AND T3.name LIKE '%foo%' LIMIT 1
         );
     ]], {
         -- <select7-4.2>
diff --git a/test/sql-tap/selectA.test.lua b/test/sql-tap/selectA.test.lua
index fc482e9..367a3f1 100755
--- a/test/sql-tap/selectA.test.lua
+++ b/test/sql-tap/selectA.test.lua
@@ -1145,7 +1145,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "selectA-2.93",
     [[
-        SELECT upper((SELECT c FROM t1 UNION SELECT z FROM t2 ORDER BY 1));
+        SELECT upper((SELECT c FROM t1 UNION SELECT z FROM t2 ORDER BY 1 LIMIT 1));
     ]], {
         -- <selectA-2.93>
         "A"
@@ -1155,7 +1155,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "selectA-2.94",
     [[
-        SELECT lower((SELECT c FROM t1 UNION ALL SELECT z FROM t2 ORDER BY 1));
+        SELECT lower((SELECT c FROM t1 UNION ALL SELECT z FROM t2 ORDER BY 1 LIMIT 1));
     ]], {
         -- <selectA-2.94>
         "a"
@@ -1165,7 +1165,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "selectA-2.95",
     [[
-        SELECT lower((SELECT c FROM t1 INTERSECT SELECT z FROM t2 ORDER BY 1));
+        SELECT lower((SELECT c FROM t1 INTERSECT SELECT z FROM t2 ORDER BY 1 LIMIT 1));
     ]], {
         -- <selectA-2.95>
         ""
@@ -1175,7 +1175,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "selectA-2.96",
     [[
-        SELECT lower((SELECT z FROM t2 EXCEPT SELECT c FROM t1 ORDER BY 1));
+        SELECT lower((SELECT z FROM t2 EXCEPT SELECT c FROM t1 ORDER BY 1 LIMIT 1));
     ]], {
         -- <selectA-2.96>
         "m"
@@ -2302,7 +2302,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "selectA-3.96",
     [[
-        SELECT lower((SELECT z FROM t2 EXCEPT SELECT c FROM t1 ORDER BY 1));
+        SELECT lower((SELECT z FROM t2 EXCEPT SELECT c FROM t1 ORDER BY 1 LIMIT 1));
     ]], {
         -- <selectA-3.96>
         "m"
@@ -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..3562c9a 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
@@ -99,7 +99,7 @@ test:do_execsql_test(
     "subselect-1.3e",
     [[
         SELECT b FROM t1
-         WHERE a = (SELECT a FROM t1 UNION SELECT b FROM t1 ORDER BY 1);
+         WHERE a = (SELECT a FROM t1 UNION SELECT b FROM t1 ORDER BY 1 LIMIT 1);
     ]], {
         -- <subselect-1.3e>
         2
@@ -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/tkt1473.test.lua b/test/sql-tap/tkt1473.test.lua
index c9b53b1..f042ca3 100755
--- a/test/sql-tap/tkt1473.test.lua
+++ b/test/sql-tap/tkt1473.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(57)
+test:plan(39)
 
 --!./tcltestrunner.lua
 -- 2005 September 19
@@ -118,17 +118,6 @@ test:do_execsql_test(
 -- Everything from this point on depends on sub-queries. So skip it
 -- if sub-queries are not available.
 
-
-test:do_execsql_test(
-    "tkt1473-2.2",
-    [[
-        SELECT (SELECT 1 FROM t1 WHERE a=1 UNION ALL SELECT 2 FROM t1 WHERE b=0)
-    ]], {
-        -- <tkt1473-2.2>
-        1
-        -- </tkt1473-2.2>
-    })
-
 test:do_execsql_test(
     "tkt1473-2.3",
     [[
@@ -140,36 +129,6 @@ test:do_execsql_test(
     })
 
 test:do_execsql_test(
-    "tkt1473-2.4",
-    [[
-        SELECT (SELECT 1 FROM t1 WHERE a=1 UNION ALL SELECT 2 FROM t1 WHERE b=4)
-    ]], {
-        -- <tkt1473-2.4>
-        1
-        -- </tkt1473-2.4>
-    })
-
-test:do_execsql_test(
-    "tkt1473-2.5",
-    [[
-        SELECT (SELECT 1 FROM t1 WHERE a=1 UNION SELECT 2 FROM t1 WHERE b=4)
-    ]], {
-        -- <tkt1473-2.5>
-        1
-        -- </tkt1473-2.5>
-    })
-
-test:do_execsql_test(
-    "tkt1473-2.6",
-    [[
-        SELECT (SELECT 1 FROM t1 WHERE a=0 UNION ALL SELECT 2 FROM t1 WHERE b=4)
-    ]], {
-        -- <tkt1473-2.6>
-        2
-        -- </tkt1473-2.6>
-    })
-
-test:do_execsql_test(
     "tkt1473-2.7",
     [[
         SELECT (SELECT 1 FROM t1 WHERE a=0 UNION SELECT 2 FROM t1 WHERE b=4)
@@ -200,17 +159,6 @@ test:do_execsql_test(
     })
 
 test:do_execsql_test(
-    "tkt1473-3.2",
-    [[
-        SELECT EXISTS
-          (SELECT 1 FROM t1 WHERE a=1 UNION ALL SELECT 2 FROM t1 WHERE b=0)
-    ]], {
-        -- <tkt1473-3.2>
-        1
-        -- </tkt1473-3.2>
-    })
-
-test:do_execsql_test(
     "tkt1473-3.3",
     [[
         SELECT EXISTS
@@ -222,39 +170,6 @@ test:do_execsql_test(
     })
 
 test:do_execsql_test(
-    "tkt1473-3.4",
-    [[
-        SELECT EXISTS
-          (SELECT 1 FROM t1 WHERE a=1 UNION ALL SELECT 2 FROM t1 WHERE b=4)
-    ]], {
-        -- <tkt1473-3.4>
-        1
-        -- </tkt1473-3.4>
-    })
-
-test:do_execsql_test(
-    "tkt1473-3.5",
-    [[
-        SELECT EXISTS
-          (SELECT 1 FROM t1 WHERE a=1 UNION SELECT 2 FROM t1 WHERE b=4)
-    ]], {
-        -- <tkt1473-3.5>
-        1
-        -- </tkt1473-3.5>
-    })
-
-test:do_execsql_test(
-    "tkt1473-3.6",
-    [[
-        SELECT EXISTS
-          (SELECT 1 FROM t1 WHERE a=0 UNION ALL SELECT 2 FROM t1 WHERE b=4)
-    ]], {
-        -- <tkt1473-3.6>
-        1
-        -- </tkt1473-3.6>
-    })
-
-test:do_execsql_test(
     "tkt1473-3.7",
     [[
         SELECT EXISTS
@@ -334,126 +249,6 @@ test:do_execsql_test(
     })
 
 test:do_execsql_test(
-    "tkt1473-4.3",
-    [[
-        SELECT (
-          SELECT 1 FROM t2 WHERE x=0
-          UNION ALL
-          SELECT 2 FROM t2 WHERE x=1
-          UNION ALL
-          SELECT 3 FROM t2 WHERE x=2
-          UNION ALL
-          SELECT 4 FROM t2 WHERE x=3
-          UNION ALL
-          SELECT 5 FROM t2 WHERE x=4
-          UNION ALL
-          SELECT 6 FROM t2 WHERE y=0
-          UNION ALL
-          SELECT 7 FROM t2 WHERE y=1
-          UNION ALL
-          SELECT 8 FROM t2 WHERE y=2
-          UNION ALL
-          SELECT 9 FROM t2 WHERE y=3
-          UNION ALL
-          SELECT 10 FROM t2 WHERE y=4
-        )
-    ]], {
-        -- <tkt1473-4.3>
-        2
-        -- </tkt1473-4.3>
-    })
-
-test:do_execsql_test(
-    "tkt1473-4.4",
-    [[
-        SELECT (
-          SELECT 1 FROM t2 WHERE x=0
-          UNION ALL
-          SELECT 2 FROM t2 WHERE x=-1
-          UNION ALL
-          SELECT 3 FROM t2 WHERE x=2
-          UNION ALL
-          SELECT 4 FROM t2 WHERE x=3
-          UNION ALL
-          SELECT 5 FROM t2 WHERE x=4
-          UNION ALL
-          SELECT 6 FROM t2 WHERE y=0
-          UNION ALL
-          SELECT 7 FROM t2 WHERE y=1
-          UNION ALL
-          SELECT 8 FROM t2 WHERE y=2
-          UNION ALL
-          SELECT 9 FROM t2 WHERE y=3
-          UNION ALL
-          SELECT 10 FROM t2 WHERE y=4
-        )
-    ]], {
-        -- <tkt1473-4.4>
-        4
-        -- </tkt1473-4.4>
-    })
-
-test:do_execsql_test(
-    "tkt1473-4.5",
-    [[
-        SELECT (
-          SELECT 1 FROM t2 WHERE x=0
-          UNION ALL
-          SELECT 2 FROM t2 WHERE x=-1
-          UNION ALL
-          SELECT 3 FROM t2 WHERE x=2
-          UNION ALL
-          SELECT 4 FROM t2 WHERE x=-1
-          UNION ALL
-          SELECT 5 FROM t2 WHERE x=4
-          UNION ALL
-          SELECT 6 FROM t2 WHERE y=0
-          UNION ALL
-          SELECT 7 FROM t2 WHERE y=1
-          UNION ALL
-          SELECT 8 FROM t2 WHERE y=2
-          UNION ALL
-          SELECT 9 FROM t2 WHERE y=3
-          UNION ALL
-          SELECT 10 FROM t2 WHERE y=-4
-        )
-    ]], {
-        -- <tkt1473-4.5>
-        8
-        -- </tkt1473-4.5>
-    })
-
-test:do_execsql_test(
-    "tkt1473-4.6",
-    [[
-        SELECT (
-          SELECT 1 FROM t2 WHERE x=0
-          UNION ALL
-          SELECT 2 FROM t2 WHERE x=-1
-          UNION ALL
-          SELECT 3 FROM t2 WHERE x=2
-          UNION ALL
-          SELECT 4 FROM t2 WHERE x=-2
-          UNION ALL
-          SELECT 5 FROM t2 WHERE x=4
-          UNION ALL
-          SELECT 6 FROM t2 WHERE y=0
-          UNION ALL
-          SELECT 7 FROM t2 WHERE y=1
-          UNION ALL
-          SELECT 8 FROM t2 WHERE y=-3
-          UNION ALL
-          SELECT 9 FROM t2 WHERE y=3
-          UNION ALL
-          SELECT 10 FROM t2 WHERE y=4
-        )
-    ]], {
-        -- <tkt1473-4.6>
-        10
-        -- </tkt1473-4.6>
-    })
-
-test:do_execsql_test(
     "tkt1473-4.7",
     [[
         SELECT (
@@ -484,126 +279,6 @@ test:do_execsql_test(
     })
 
 test:do_execsql_test(
-    "tkt1473-5.3",
-    [[
-        SELECT EXISTS (
-          SELECT 1 FROM t2 WHERE x=0
-          UNION ALL
-          SELECT 2 FROM t2 WHERE x=1
-          UNION ALL
-          SELECT 3 FROM t2 WHERE x=2
-          UNION ALL
-          SELECT 4 FROM t2 WHERE x=3
-          UNION ALL
-          SELECT 5 FROM t2 WHERE x=4
-          UNION ALL
-          SELECT 6 FROM t2 WHERE y=0
-          UNION ALL
-          SELECT 7 FROM t2 WHERE y=1
-          UNION ALL
-          SELECT 8 FROM t2 WHERE y=2
-          UNION ALL
-          SELECT 9 FROM t2 WHERE y=3
-          UNION ALL
-          SELECT 10 FROM t2 WHERE y=4
-        )
-    ]], {
-        -- <tkt1473-5.3>
-        1
-        -- </tkt1473-5.3>
-    })
-
-test:do_execsql_test(
-    "tkt1473-5.4",
-    [[
-        SELECT EXISTS (
-          SELECT 1 FROM t2 WHERE x=0
-          UNION ALL
-          SELECT 2 FROM t2 WHERE x=-1
-          UNION ALL
-          SELECT 3 FROM t2 WHERE x=2
-          UNION ALL
-          SELECT 4 FROM t2 WHERE x=3
-          UNION ALL
-          SELECT 5 FROM t2 WHERE x=4
-          UNION ALL
-          SELECT 6 FROM t2 WHERE y=0
-          UNION ALL
-          SELECT 7 FROM t2 WHERE y=1
-          UNION ALL
-          SELECT 8 FROM t2 WHERE y=2
-          UNION ALL
-          SELECT 9 FROM t2 WHERE y=3
-          UNION ALL
-          SELECT 10 FROM t2 WHERE y=4
-        )
-    ]], {
-        -- <tkt1473-5.4>
-        1
-        -- </tkt1473-5.4>
-    })
-
-test:do_execsql_test(
-    "tkt1473-5.5",
-    [[
-        SELECT EXISTS (
-          SELECT 1 FROM t2 WHERE x=0
-          UNION ALL
-          SELECT 2 FROM t2 WHERE x=-1
-          UNION ALL
-          SELECT 3 FROM t2 WHERE x=2
-          UNION ALL
-          SELECT 4 FROM t2 WHERE x=-1
-          UNION ALL
-          SELECT 5 FROM t2 WHERE x=4
-          UNION ALL
-          SELECT 6 FROM t2 WHERE y=0
-          UNION ALL
-          SELECT 7 FROM t2 WHERE y=1
-          UNION ALL
-          SELECT 8 FROM t2 WHERE y=2
-          UNION ALL
-          SELECT 9 FROM t2 WHERE y=3
-          UNION ALL
-          SELECT 10 FROM t2 WHERE y=-4
-        )
-    ]], {
-        -- <tkt1473-5.5>
-        1
-        -- </tkt1473-5.5>
-    })
-
-test:do_execsql_test(
-    "tkt1473-5.6",
-    [[
-        SELECT EXISTS (
-          SELECT 1 FROM t2 WHERE x=0
-          UNION ALL
-          SELECT 2 FROM t2 WHERE x=-1
-          UNION ALL
-          SELECT 3 FROM t2 WHERE x=2
-          UNION ALL
-          SELECT 4 FROM t2 WHERE x=-2
-          UNION ALL
-          SELECT 5 FROM t2 WHERE x=4
-          UNION ALL
-          SELECT 6 FROM t2 WHERE y=0
-          UNION ALL
-          SELECT 7 FROM t2 WHERE y=1
-          UNION ALL
-          SELECT 8 FROM t2 WHERE y=-3
-          UNION ALL
-          SELECT 9 FROM t2 WHERE y=3
-          UNION ALL
-          SELECT 10 FROM t2 WHERE y=4
-        )
-    ]], {
-        -- <tkt1473-5.6>
-        1
-        -- </tkt1473-5.6>
-    })
-
-test:do_execsql_test(
     "tkt1473-5.7",
     [[
         SELECT EXISTS (
@@ -634,66 +309,6 @@ test:do_execsql_test(
     })
 
 test:do_execsql_test(
-    "tkt1473-6.3",
-    [[
-        SELECT EXISTS (
-          SELECT 1 FROM t2 WHERE x=0
-          UNION
-          SELECT 2 FROM t2 WHERE x=1
-          UNION
-          SELECT 3 FROM t2 WHERE x=2
-          UNION
-          SELECT 4 FROM t2 WHERE x=3
-          UNION
-          SELECT 5 FROM t2 WHERE x=4
-          UNION
-          SELECT 6 FROM t2 WHERE y=0
-          UNION
-          SELECT 7 FROM t2 WHERE y=1
-          UNION
-          SELECT 8 FROM t2 WHERE y=2
-          UNION
-          SELECT 9 FROM t2 WHERE y=3
-          UNION
-          SELECT 10 FROM t2 WHERE y=4
-        )
-    ]], {
-        -- <tkt1473-6.3>
-        1
-        -- </tkt1473-6.3>
-    })
-
-test:do_execsql_test(
-    "tkt1473-6.4",
-    [[
-        SELECT EXISTS (
-          SELECT 1 FROM t2 WHERE x=0
-          UNION
-          SELECT 2 FROM t2 WHERE x=-1
-          UNION
-          SELECT 3 FROM t2 WHERE x=2
-          UNION
-          SELECT 4 FROM t2 WHERE x=3
-          UNION
-          SELECT 5 FROM t2 WHERE x=4
-          UNION
-          SELECT 6 FROM t2 WHERE y=0
-          UNION
-          SELECT 7 FROM t2 WHERE y=1
-          UNION
-          SELECT 8 FROM t2 WHERE y=2
-          UNION
-          SELECT 9 FROM t2 WHERE y=3
-          UNION
-          SELECT 10 FROM t2 WHERE y=4
-        )
-    ]], {
-        -- <tkt1473-6.4>
-        1
-        -- </tkt1473-6.4>
-    })
-
-test:do_execsql_test(
     "tkt1473-6.5",
     [[
         SELECT EXISTS (
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;
diff --git a/test/sql/gh-2366-whith-select-subquery.result b/test/sql/gh-2366-whith-select-subquery.result
new file mode 100644
index 0000000..e6edf2b
--- /dev/null
+++ b/test/sql/gh-2366-whith-select-subquery.result
@@ -0,0 +1,56 @@
+env = require('test_run')
+---
+...
+test_run = env.new()
+---
+...
+--
+-- gh-2366: SQL subquery with =
+--
+box.sql.execute("CREATE TABLE t1(a int primary key, b int);")
+---
+...
+box.sql.execute("INSERT INTO t1 VALUES(1,2);")
+---
+...
+box.sql.execute("INSERT INTO t1 VALUES(3,4);")
+---
+...
+box.sql.execute("INSERT INTO t1 VALUES(5,6);")
+---
+...
+box.sql.execute("INSERT INTO t1 VALUES(7,6);")
+---
+...
+box.sql.execute("select * from t1;")
+---
+- - [1, 2]
+  - [3, 4]
+  - [5, 6]
+  - [7, 6]
+...
+box.sql.execute("SELECT a FROM t1 WHERE b=6 LIMIT (select b from t1 where a =1);")
+---
+- - [5]
+  - [7]
+...
+box.sql.execute("SELECT b from t1 where a = (SELECT a FROM t1 WHERE b=6);")
+---
+- error: SELECT subquery returned more than 1 row
+...
+box.sql.execute("SELECT b from t1 where a = (SELECT a FROM t1 WHERE b=6 LIMIT (select b-1 from t1 where a =1));")
+---
+- - [6]
+...
+box.sql.execute("SELECT b from t1 where a = (SELECT a FROM t1 WHERE b=6 LIMIT (select b from t1 where a =1));")
+---
+- error: SELECT subquery could be only limited with 1
+...
+box.sql.execute("SELECT (VALUES(1),(2),(3),(4));")
+---
+- error: SELECT subquery returned more than 1 row
+...
+-- clean-up
+box.sql.execute("DROP TABLE t1;")
+---
+...
diff --git a/test/sql/gh-2366-whith-select-subquery.test.lua b/test/sql/gh-2366-whith-select-subquery.test.lua
new file mode 100644
index 0000000..39116df
--- /dev/null
+++ b/test/sql/gh-2366-whith-select-subquery.test.lua
@@ -0,0 +1,23 @@
+env = require('test_run')
+test_run = env.new()
+
+--
+-- gh-2366: SQL subquery with =
+--
+
+box.sql.execute("CREATE TABLE t1(a int primary key, b int);")
+box.sql.execute("INSERT INTO t1 VALUES(1,2);")
+box.sql.execute("INSERT INTO t1 VALUES(3,4);")
+box.sql.execute("INSERT INTO t1 VALUES(5,6);")
+box.sql.execute("INSERT INTO t1 VALUES(7,6);")
+
+box.sql.execute("select * from t1;")
+box.sql.execute("SELECT a FROM t1 WHERE b=6 LIMIT (select b from t1 where a =1);")
+box.sql.execute("SELECT b from t1 where a = (SELECT a FROM t1 WHERE b=6);")
+box.sql.execute("SELECT b from t1 where a = (SELECT a FROM t1 WHERE b=6 LIMIT (select b-1 from t1 where a =1));")
+box.sql.execute("SELECT b from t1 where a = (SELECT a FROM t1 WHERE b=6 LIMIT (select b from t1 where a =1));")
+
+box.sql.execute("SELECT (VALUES(1),(2),(3),(4));")
+
+-- clean-up
+box.sql.execute("DROP TABLE t1;")
\ No newline at end of file
-- 
2.7.4

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [tarantool-patches] Re: [PATCH v2 1/1] sql: disallow returning many rows from subselect
  2018-06-27 15:40   ` Kirill Shcherbatov
@ 2018-06-28 13:04     ` n.pettik
  2018-06-28 17:08       ` Kirill Shcherbatov
  0 siblings, 1 reply; 15+ messages in thread
From: n.pettik @ 2018-06-28 13:04 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Kirill Shcherbatov

Thanks for the patch. I can provide only few minor comments.

> diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
> index 70e134f..23cee59 100644
> --- a/src/box/sql/expr.c
> +++ b/src/box/sql/expr.c
> @@ -2874,10 +2874,14 @@ 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 =
> +					sqlite3ExprAlloc(pParse->db, TK_INTEGER,
> +							 &sqlite3IntTokens[1],
> +							 0);

Should we test pLimit on OOM? AFAIS sqlite3ExprAlloc can return NULL ptr.

> +				ExprSetProperty(pSel->pLimit, EP_System);
> +			}
> +			pSel->selFlags |= SF_SingleRow;
> 			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 4e61ec1..e904c2a 100644
> --- a/src/box/sql/select.c
> +++ b/src/box/sql/select.c
> @@ -2119,6 +2119,38 @@ computeLimitRegisters(Parse * pParse, Select * p, int iBreak)
> 			sqlite3VdbeAddOp2(v, OP_IfNot, iLimit, iBreak);
> 			VdbeCoverage(v);
> 		}
> +		if (p->selFlags & SF_SingleRow) {
> +			if (ExprHasProperty(p->pLimit, EP_System)) {
> +				/*
> +				 * Indirect LIMIT 1 is allowed only for
> +				 * requests returning only 1 row.
> +				 * To test this, we change LIMIT 1 to
> +				 * LIMIT 2 and will look up LIMIT 1 overflow
> +				 * at the sqlite3Select end.
> +				 */
> +				sqlite3VdbeAddOp2(v, OP_Integer, 2, iLimit);
> +			} else {
> +				/*
> +				 * User-defined complex limit for subquery
> +				 * could be only 1 as resulting value.
> +				 */
> +				int r1 = sqlite3GetTempReg(pParse);
> +				sqlite3VdbeAddOp2(v, OP_Integer, 1, r1);
> +				int no_err = sqlite3VdbeMakeLabel(v);
> +				sqlite3VdbeAddOp3(v, OP_Eq, iLimit, no_err, r1);
> +				const char *error =
> +					"SELECT subquery could be only limited "
> +					"with 1”;

Lets remove ’SELECT’ word: subquery implies it.

“Expression subquery could be limited only with 1.” -
I suggest this variant.

> +				sqlite3VdbeAddOp4(v, OP_Halt,
> +						  SQL_TARANTOOL_ERROR,
> +						  0, 0, error, P4_STATIC);
> +				sqlite3VdbeResolveLabel(v, no_err);
> +				sqlite3ReleaseTempReg(pParse, r1);
> +
> +				/* Runtime checks are no longer needed. */
> +				p->selFlags &= ~SF_SingleRow;
> +			}
> +		}
> 		if (p->pOffset) {
> 			p->iOffset = iOffset = ++pParse->nMem;
> 			pParse->nMem++;	/* Allocate an extra register for limit+offset */
> @@ -5368,6 +5400,23 @@ explain_simple_count(struct Parse *parse_context, const char *table_name)
> 	}
> }
> 

AFAIR Kostya and Vlad ask for adding comments even for static functions.

> +static void
> +vdbe_code_raise_on_multiple_rows(struct Parse *parser, int limit_reg, int end_mark)
> +{
> +	assert(limit_reg != 0);
> +	struct Vdbe *v = sqlite3GetVdbe(parser);
> +	assert(v != NULL);
> +
> +	int r1 = sqlite3GetTempReg(parser);
> +	sqlite3VdbeAddOp2(v, OP_Integer, 0, r1);
> +	sqlite3VdbeAddOp3(v, OP_Ne, r1, end_mark, limit_reg);
> +	const char *error = "SELECT subquery returned more than 1 row”;

Again: most DBs use next variant:

“Expression subquery returned more than 1 row” OR
“More than one row returned by a subquery used as an expression”

Btw, last variant is used in PostgeSQL 

> +	sqlite3VdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR,
> +			  ON_CONFLICT_ACTION_FAIL, 0,
> +			  error, P4_STATIC);
> +	sqlite3ReleaseTempReg(parser, r1);
> +}
> +
> /*
>  * Generate code for the SELECT statement given in the p argument.
>  *
> @@ -5504,6 +5553,12 @@ sqlite3Select(Parse * pParse,		/* The parser context */
> 	if (p->pPrior) {
> 		rc = multiSelect(pParse, p, pDest);
> 		pParse->iSelectId = iRestoreSelectId;
> +
> +		int end = sqlite3VdbeMakeLabel(v);
> +		if (p->selFlags & SF_SingleRow && p->iLimit != 0)
> +			vdbe_code_raise_on_multiple_rows(pParse, p->iLimit, end);
> +		sqlite3VdbeResolveLabel(v, end);
> +
> #ifdef SELECTTRACE_ENABLED
> 		SELECTTRACE(1, pParse, p, ("end compound-select processing\n"));
> 		pParse->nSelectIndent--;
> @@ -6296,8 +6351,10 @@ 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. */
> +	if (p->selFlags & SF_SingleRow && p->iLimit != 0)
> +		vdbe_code_raise_on_multiple_rows(pParse, p->iLimit, iEnd);
> +	/* 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 5f7a0f1..093aabb 100644
> --- a/src/box/sql/sqliteInt.h
> +++ b/src/box/sql/sqliteInt.h
> @@ -2370,6 +2370,8 @@ struct Expr {
> #define EP_Subquery  0x200000	/* Tree contains a TK_SELECT operator */
> #define EP_Alias     0x400000	/* Is an alias for a result set column */
> #define EP_Leaf      0x800000	/* Expr.pLeft, .pRight, .u.pSelect all NULL */
> +/** Expression is system-defined. */
> +#define EP_System    0x1000000
> 
> /*
>  * Combinations of two or more EP_* flags
> @@ -2718,6 +2720,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. */

Typo: ‘contains’.

> diff --git a/test/sql-tap/select4.test.lua b/test/sql-tap/select4.test.lua
> index 5f91b13..5b49d8b 100755
> --- a/test/sql-tap/select4.test.lua
> +++ b/test/sql-tap/select4.test.lua
> @@ -1,6 +1,6 @@
> #!/usr/bin/env tarantool
> test = require("sqltester")
> -test:plan(103)
> +test:plan(101)
> 
> --!./tcltestrunner.lua
> -- 2001 September 15
> @@ -1379,26 +1379,6 @@ test:do_execsql_test(
>     })
> 
> test:do_execsql_test(
> -    "select4-14.10",
> -    [[
> -        SELECT (VALUES(1),(2),(3),(4))
> -    ]], {
> -        -- <select4-14.10>
> -        1
> -        -- </select4-14.10>
> -    })

I wouldn’t delete these tests: let them remain to check that
expression subqueries tolerate only one row as result.
So, simply change type of test to catchsql and fix its result.

It also related to the rest of deleted tests (tkt-473 and so on).

> diff --git a/test/sql/gh-2366-whith-select-subquery.test.lua b/test/sql/gh-2366-whith-select-subquery.test.lua
> new file mode 100644
> index 0000000..39116df
> --- /dev/null
> +++ b/test/sql/gh-2366-whith-select-subquery.test.lua

I wouldn’t create separate sql/ test for this issue:
there are a lot of sql-tap/ tests on subqueries - you are able
to move these tests to one of them.

> @@ -0,0 +1,23 @@
> +env = require('test_run')
> +test_run = env.new()
> +
> +--
> +-- gh-2366: SQL subquery with =
> +--
> +
> +box.sql.execute("CREATE TABLE t1(a int primary key, b int);")
> +box.sql.execute("INSERT INTO t1 VALUES(1,2);")
> +box.sql.execute("INSERT INTO t1 VALUES(3,4);")
> +box.sql.execute("INSERT INTO t1 VALUES(5,6);")
> +box.sql.execute("INSERT INTO t1 VALUES(7,6);")
> +
> +box.sql.execute("select * from t1;”)

Lets use upper-case for all SQL statements.

> +box.sql.execute("SELECT a FROM t1 WHERE b=6 LIMIT (select b from t1 where a =1);")
> +box.sql.execute("SELECT b from t1 where a = (SELECT a FROM t1 WHERE b=6);")
> +box.sql.execute("SELECT b from t1 where a = (SELECT a FROM t1 WHERE b=6 LIMIT (select b-1 from t1 where a =1));")
> +box.sql.execute("SELECT b from t1 where a = (SELECT a FROM t1 WHERE b=6 LIMIT (select b from t1 where a =1));")
> +
> +box.sql.execute("SELECT (VALUES(1),(2),(3),(4));")
> +
> +-- clean-up
> +box.sql.execute("DROP TABLE t1;")
> \ No newline at end of file

Put new line at end of file.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [tarantool-patches] Re: [PATCH v2 1/1] sql: disallow returning many rows from subselect
  2018-06-28 13:04     ` [tarantool-patches] " n.pettik
@ 2018-06-28 17:08       ` Kirill Shcherbatov
  2018-06-29 13:15         ` n.pettik
  0 siblings, 1 reply; 15+ messages in thread
From: Kirill Shcherbatov @ 2018-06-28 17:08 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Nikita Pettik

> Should we test pLimit on OOM? AFAIS sqlite3ExprAlloc can return NULL ptr.
It is useless, as at the beginning if the sqlite3Select oom check returning -1 on invalid Parser state.

> “Expression subquery could be limited only with 1.” -
> I suggest this variant.
-                                       "SELECT subquery could be only limited "
-                                       "with 1";
+                                       "Expression subquery could be limited "
+                                       "only with 1.";

> AFAIR Kostya and Vlad ask for adding comments even for static functions.
+/**
+ * Generate VDBE code that HALT program when subselect returned
+ * more than one row (determined as LIMIT 1 overflow).
+ * @param parser Current parsing context.
+ * @param limit_reg LIMIT register.
+ * @param end_mark mark to jump if select returned distinct one
+ *                 row as expected.
+ */

> “Expression subquery returned more than 1 row” OR
> “More than one row returned by a subquery used as an expression”
-       const char *error = "SELECT subquery returned more than 1 row";
+       const char *error = "Expression subquery returned more than 1 row";


> Typo: ‘contains’.
-/** Abort subquery if its output contain more than one row. */
+/** Abort subquery if its output contains more than one row. */


> I wouldn’t delete these tests: let them remain to check that
> expression subqueries tolerate only one row as result.
> So, simply change type of test to catchsql and fix its result.
> 
> It also related to the rest of deleted tests (tkt-473 and so on).
-test:do_execsql_test(
+test:do_catchsql_test(
     "select4-14.10",
     [[
         SELECT (VALUES(1),(2),(3),(4))
     ]], {
         -- <select4-14.10>
-        1
+        1, "Expression subquery returned more than 1 row"
         -- </select4-14.10>
     })

> I wouldn’t create separate sql/ test for this issue:
> there are a lot of sql-tap/ tests on subqueries - you are able
> to move these tests to one of them.
Ok, not a problem.

> Lets use upper-case for all SQL statements.
> Put new line at end of file.
Have file - have problem. No file - no problem.
=======================================================

To follow ANSI SQL standard we should dissallow returning
multiple rows from subselects after = and in braces ().
To achieve this goal we have introduced 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 extra bytecode to HALT execution on
reaching this restrict.

Resolves #2366.
---
 src/box/sql/expr.c              | 12 ++++---
 src/box/sql/select.c            | 69 +++++++++++++++++++++++++++++++++++++--
 src/box/sql/sqliteInt.h         |  4 +++
 test/sql-tap/aggnested.test.lua |  2 +-
 test/sql-tap/e_expr.test.lua    | 42 ++++++++++++------------
 test/sql-tap/misc5.test.lua     | 15 ++++-----
 test/sql-tap/select4.test.lua   |  8 ++---
 test/sql-tap/select7.test.lua   |  2 +-
 test/sql-tap/selectA.test.lua   | 14 ++++----
 test/sql-tap/subquery.test.lua  | 14 ++++----
 test/sql-tap/subquery2.test.lua |  4 +--
 test/sql-tap/subselect.test.lua | 54 ++++++++++++++++++++++++++++---
 test/sql-tap/tkt1473.test.lua   | 72 ++++++++++++++++++++---------------------
 test/sql-tap/with1.test.lua     |  2 +-
 14 files changed, 214 insertions(+), 100 deletions(-)

diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index 70e134f..23cee59 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -2874,10 +2874,14 @@ 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 =
+					sqlite3ExprAlloc(pParse->db, TK_INTEGER,
+							 &sqlite3IntTokens[1],
+							 0);
+				ExprSetProperty(pSel->pLimit, EP_System);
+			}
+			pSel->selFlags |= SF_SingleRow;
 			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 4e61ec1..c03e960 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -2119,6 +2119,38 @@ computeLimitRegisters(Parse * pParse, Select * p, int iBreak)
 			sqlite3VdbeAddOp2(v, OP_IfNot, iLimit, iBreak);
 			VdbeCoverage(v);
 		}
+		if (p->selFlags & SF_SingleRow) {
+			if (ExprHasProperty(p->pLimit, EP_System)) {
+				/*
+				 * Indirect LIMIT 1 is allowed only for
+				 * requests returning only 1 row.
+				 * To test this, we change LIMIT 1 to
+				 * LIMIT 2 and will look up LIMIT 1 overflow
+				 * at the sqlite3Select end.
+				 */
+				sqlite3VdbeAddOp2(v, OP_Integer, 2, iLimit);
+			} else {
+				/*
+				 * User-defined complex limit for subquery
+				 * could be only 1 as resulting value.
+				 */
+				int r1 = sqlite3GetTempReg(pParse);
+				sqlite3VdbeAddOp2(v, OP_Integer, 1, r1);
+				int no_err = sqlite3VdbeMakeLabel(v);
+				sqlite3VdbeAddOp3(v, OP_Eq, iLimit, no_err, r1);
+				const char *error =
+					"Expression subquery could be limited "
+					"only with 1.";
+				sqlite3VdbeAddOp4(v, OP_Halt,
+						  SQL_TARANTOOL_ERROR,
+						  0, 0, error, P4_STATIC);
+				sqlite3VdbeResolveLabel(v, no_err);
+				sqlite3ReleaseTempReg(pParse, r1);
+
+				/* Runtime checks are no longer needed. */
+				p->selFlags &= ~SF_SingleRow;
+			}
+		}
 		if (p->pOffset) {
 			p->iOffset = iOffset = ++pParse->nMem;
 			pParse->nMem++;	/* Allocate an extra register for limit+offset */
@@ -5368,6 +5400,31 @@ explain_simple_count(struct Parse *parse_context, const char *table_name)
 	}
 }
 
+/**
+ * Generate VDBE code that HALT program when subselect returned
+ * more than one row (determined as LIMIT 1 overflow).
+ * @param parser Current parsing context.
+ * @param limit_reg LIMIT register.
+ * @param end_mark mark to jump if select returned distinct one
+ *                 row as expected.
+ */
+static void
+vdbe_code_raise_on_multiple_rows(struct Parse *parser, int limit_reg, int end_mark)
+{
+	assert(limit_reg != 0);
+	struct Vdbe *v = sqlite3GetVdbe(parser);
+	assert(v != NULL);
+
+	int r1 = sqlite3GetTempReg(parser);
+	sqlite3VdbeAddOp2(v, OP_Integer, 0, r1);
+	sqlite3VdbeAddOp3(v, OP_Ne, r1, end_mark, limit_reg);
+	const char *error = "Expression subquery returned more than 1 row";
+	sqlite3VdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR,
+			  ON_CONFLICT_ACTION_FAIL, 0,
+			  error, P4_STATIC);
+	sqlite3ReleaseTempReg(parser, r1);
+}
+
 /*
  * Generate code for the SELECT statement given in the p argument.
  *
@@ -5504,6 +5561,12 @@ sqlite3Select(Parse * pParse,		/* The parser context */
 	if (p->pPrior) {
 		rc = multiSelect(pParse, p, pDest);
 		pParse->iSelectId = iRestoreSelectId;
+
+		int end = sqlite3VdbeMakeLabel(v);
+		if (p->selFlags & SF_SingleRow && p->iLimit != 0)
+			vdbe_code_raise_on_multiple_rows(pParse, p->iLimit, end);
+		sqlite3VdbeResolveLabel(v, end);
+
 #ifdef SELECTTRACE_ENABLED
 		SELECTTRACE(1, pParse, p, ("end compound-select processing\n"));
 		pParse->nSelectIndent--;
@@ -6296,8 +6359,10 @@ 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. */
+	if (p->selFlags & SF_SingleRow && p->iLimit != 0)
+		vdbe_code_raise_on_multiple_rows(pParse, p->iLimit, iEnd);
+	/* 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 68471b7..ccb7cc6 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -2370,6 +2370,8 @@ struct Expr {
 #define EP_Subquery  0x200000	/* Tree contains a TK_SELECT operator */
 #define EP_Alias     0x400000	/* Is an alias for a result set column */
 #define EP_Leaf      0x800000	/* Expr.pLeft, .pRight, .u.pSelect all NULL */
+/** Expression is system-defined. */
+#define EP_System    0x1000000
 
 /*
  * Combinations of two or more EP_* flags
@@ -2718,6 +2720,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 contains 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..8525ce2 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,12 +3474,12 @@ 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)
+  (SELECT b FROM t22 UNION SELECT a+1 FROM t22 LIMIT 1)
 ]], "null", "")
 do_expr_test("e_expr-35.1.6", [[ 
-  (SELECT a FROM t22 UNION SELECT COALESCE(b, 55) FROM t22 ORDER BY 1)
+  (SELECT a FROM t22 UNION SELECT COALESCE(b, 55) FROM t22 ORDER BY 1 LIMIT 1)
 ]], "integer", 4)
 -- EVIDENCE-OF: R-46899-53765 A SELECT used as a scalar quantity must
 -- return a result set with a single column.
@@ -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/select4.test.lua b/test/sql-tap/select4.test.lua
index 5f91b13..d88ea4c 100755
--- a/test/sql-tap/select4.test.lua
+++ b/test/sql-tap/select4.test.lua
@@ -1378,23 +1378,23 @@ test:do_execsql_test(
         -- </select4-14.9>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "select4-14.10",
     [[
         SELECT (VALUES(1),(2),(3),(4))
     ]], {
         -- <select4-14.10>
-        1
+        1, "Expression subquery returned more than 1 row"
         -- </select4-14.10>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "select4-14.11",
     [[
         SELECT (SELECT 1 UNION ALL SELECT 2 UNION ALL SELECT 3 UNION ALL SELECT 4)
     ]], {
         -- <select4-14.11>
-        1
+        1, "Expression subquery returned more than 1 row"
         -- </select4-14.11>
     })
 
diff --git a/test/sql-tap/select7.test.lua b/test/sql-tap/select7.test.lua
index 7fc0a3c..10e13e2 100755
--- a/test/sql-tap/select7.test.lua
+++ b/test/sql-tap/select7.test.lua
@@ -112,7 +112,7 @@ test:do_execsql_test(
         SELECT P.pk from PHOTO P WHERE NOT EXISTS (
              SELECT T2.pk from TAG T2 WHERE T2.fk = P.pk
              EXCEPT
-             SELECT T3.pk from TAG T3 WHERE T3.fk = P.pk AND T3.name LIKE '%foo%'
+             SELECT T3.pk from TAG T3 WHERE T3.fk = P.pk AND T3.name LIKE '%foo%' LIMIT 1
         );
     ]], {
         -- <select7-4.2>
diff --git a/test/sql-tap/selectA.test.lua b/test/sql-tap/selectA.test.lua
index fc482e9..367a3f1 100755
--- a/test/sql-tap/selectA.test.lua
+++ b/test/sql-tap/selectA.test.lua
@@ -1145,7 +1145,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "selectA-2.93",
     [[
-        SELECT upper((SELECT c FROM t1 UNION SELECT z FROM t2 ORDER BY 1));
+        SELECT upper((SELECT c FROM t1 UNION SELECT z FROM t2 ORDER BY 1 LIMIT 1));
     ]], {
         -- <selectA-2.93>
         "A"
@@ -1155,7 +1155,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "selectA-2.94",
     [[
-        SELECT lower((SELECT c FROM t1 UNION ALL SELECT z FROM t2 ORDER BY 1));
+        SELECT lower((SELECT c FROM t1 UNION ALL SELECT z FROM t2 ORDER BY 1 LIMIT 1));
     ]], {
         -- <selectA-2.94>
         "a"
@@ -1165,7 +1165,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "selectA-2.95",
     [[
-        SELECT lower((SELECT c FROM t1 INTERSECT SELECT z FROM t2 ORDER BY 1));
+        SELECT lower((SELECT c FROM t1 INTERSECT SELECT z FROM t2 ORDER BY 1 LIMIT 1));
     ]], {
         -- <selectA-2.95>
         ""
@@ -1175,7 +1175,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "selectA-2.96",
     [[
-        SELECT lower((SELECT z FROM t2 EXCEPT SELECT c FROM t1 ORDER BY 1));
+        SELECT lower((SELECT z FROM t2 EXCEPT SELECT c FROM t1 ORDER BY 1 LIMIT 1));
     ]], {
         -- <selectA-2.96>
         "m"
@@ -2302,7 +2302,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "selectA-3.96",
     [[
-        SELECT lower((SELECT z FROM t2 EXCEPT SELECT c FROM t1 ORDER BY 1));
+        SELECT lower((SELECT z FROM t2 EXCEPT SELECT c FROM t1 ORDER BY 1 LIMIT 1));
     ]], {
         -- <selectA-3.96>
         "m"
@@ -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..636d2f6 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(30)
 
 --!./tcltestrunner.lua
 -- 2001 September 15
@@ -99,7 +99,7 @@ test:do_execsql_test(
     "subselect-1.3e",
     [[
         SELECT b FROM t1
-         WHERE a = (SELECT a FROM t1 UNION SELECT b FROM t1 ORDER BY 1);
+         WHERE a = (SELECT a FROM t1 UNION SELECT b FROM t1 ORDER BY 1 LIMIT 1);
     ]], {
         -- <subselect-1.3e>
         2
@@ -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,50 @@ 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, "Expression 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, "Expression subquery returned more than 1 row"
+    -- </subselect-5.2>
+})
+
+test:do_execsql_test(
+    "subselect-5.3",
+    [[
+        SELECT b from t1 where a = (SELECT a FROM t1 WHERE b=6 LIMIT (select b-1 from t1 where a =1));
+    ]], {
+    -- <subselect-5.2>
+    6
+    -- </subselect-5.2>
+})
 
+test:do_catchsql_test(
+    "subselect-5.3",
+    [[
+        SELECT b from t1 where a = (SELECT a FROM t1 WHERE b=6 LIMIT (select b from t1 where a =1));
+    ]], {
+    -- <subselect-5.2>
+    1, "Expression subquery could be limited only with 1."
+    -- </subselect-5.2>
+})
 
 test:finish_test()
diff --git a/test/sql-tap/tkt1473.test.lua b/test/sql-tap/tkt1473.test.lua
index c9b53b1..a869725 100755
--- a/test/sql-tap/tkt1473.test.lua
+++ b/test/sql-tap/tkt1473.test.lua
@@ -119,13 +119,13 @@ test:do_execsql_test(
 -- if sub-queries are not available.
 
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "tkt1473-2.2",
     [[
         SELECT (SELECT 1 FROM t1 WHERE a=1 UNION ALL SELECT 2 FROM t1 WHERE b=0)
     ]], {
         -- <tkt1473-2.2>
-        1
+        1, "Expression subquery returned more than 1 row"
         -- </tkt1473-2.2>
     })
 
@@ -139,33 +139,33 @@ test:do_execsql_test(
         -- </tkt1473-2.3>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "tkt1473-2.4",
     [[
         SELECT (SELECT 1 FROM t1 WHERE a=1 UNION ALL SELECT 2 FROM t1 WHERE b=4)
     ]], {
         -- <tkt1473-2.4>
-        1
+        1, "Expression subquery returned more than 1 row"
         -- </tkt1473-2.4>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "tkt1473-2.5",
     [[
         SELECT (SELECT 1 FROM t1 WHERE a=1 UNION SELECT 2 FROM t1 WHERE b=4)
     ]], {
         -- <tkt1473-2.5>
-        1
+        1, "Expression subquery returned more than 1 row"
         -- </tkt1473-2.5>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "tkt1473-2.6",
     [[
         SELECT (SELECT 1 FROM t1 WHERE a=0 UNION ALL SELECT 2 FROM t1 WHERE b=4)
     ]], {
         -- <tkt1473-2.6>
-        2
+        1, "Expression subquery returned more than 1 row"
         -- </tkt1473-2.6>
     })
 
@@ -199,14 +199,14 @@ test:do_execsql_test(
         -- </tkt1473-2.9>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "tkt1473-3.2",
     [[
         SELECT EXISTS
           (SELECT 1 FROM t1 WHERE a=1 UNION ALL SELECT 2 FROM t1 WHERE b=0)
     ]], {
         -- <tkt1473-3.2>
-        1
+        1, "Expression subquery returned more than 1 row"
         -- </tkt1473-3.2>
     })
 
@@ -221,36 +221,36 @@ test:do_execsql_test(
         -- </tkt1473-3.3>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "tkt1473-3.4",
     [[
         SELECT EXISTS
           (SELECT 1 FROM t1 WHERE a=1 UNION ALL SELECT 2 FROM t1 WHERE b=4)
     ]], {
         -- <tkt1473-3.4>
-        1
+        1, "Expression subquery returned more than 1 row"
         -- </tkt1473-3.4>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "tkt1473-3.5",
     [[
         SELECT EXISTS
           (SELECT 1 FROM t1 WHERE a=1 UNION SELECT 2 FROM t1 WHERE b=4)
     ]], {
         -- <tkt1473-3.5>
-        1
+        1, "Expression subquery returned more than 1 row"
         -- </tkt1473-3.5>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "tkt1473-3.6",
     [[
         SELECT EXISTS
           (SELECT 1 FROM t1 WHERE a=0 UNION ALL SELECT 2 FROM t1 WHERE b=4)
     ]], {
         -- <tkt1473-3.6>
-        1
+        1, "Expression subquery returned more than 1 row"
         -- </tkt1473-3.6>
     })
 
@@ -333,7 +333,7 @@ test:do_execsql_test(
         -- </tkt1473-4.2>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "tkt1473-4.3",
     [[
         SELECT (
@@ -359,11 +359,11 @@ test:do_execsql_test(
         )
     ]], {
         -- <tkt1473-4.3>
-        2
+        1, "Expression subquery returned more than 1 row"
         -- </tkt1473-4.3>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "tkt1473-4.4",
     [[
         SELECT (
@@ -389,11 +389,11 @@ test:do_execsql_test(
         )
     ]], {
         -- <tkt1473-4.4>
-        4
+        1, "Expression subquery returned more than 1 row"
         -- </tkt1473-4.4>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "tkt1473-4.5",
     [[
         SELECT (
@@ -419,11 +419,11 @@ test:do_execsql_test(
         )
     ]], {
         -- <tkt1473-4.5>
-        8
+        1, "Expression subquery returned more than 1 row"
         -- </tkt1473-4.5>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "tkt1473-4.6",
     [[
         SELECT (
@@ -449,7 +449,7 @@ test:do_execsql_test(
         )
     ]], {
         -- <tkt1473-4.6>
-        10
+        1, "Expression subquery returned more than 1 row"
         -- </tkt1473-4.6>
     })
 
@@ -483,7 +483,7 @@ test:do_execsql_test(
         -- </tkt1473-4.7>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "tkt1473-5.3",
     [[
         SELECT EXISTS (
@@ -509,11 +509,11 @@ test:do_execsql_test(
         )
     ]], {
         -- <tkt1473-5.3>
-        1
+        1, "Expression subquery returned more than 1 row"
         -- </tkt1473-5.3>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "tkt1473-5.4",
     [[
         SELECT EXISTS (
@@ -539,11 +539,11 @@ test:do_execsql_test(
         )
     ]], {
         -- <tkt1473-5.4>
-        1
+        1, "Expression subquery returned more than 1 row"
         -- </tkt1473-5.4>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "tkt1473-5.5",
     [[
         SELECT EXISTS (
@@ -569,11 +569,11 @@ test:do_execsql_test(
         )
     ]], {
         -- <tkt1473-5.5>
-        1
+        1, "Expression subquery returned more than 1 row"
         -- </tkt1473-5.5>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "tkt1473-5.6",
     [[
         SELECT EXISTS (
@@ -599,7 +599,7 @@ test:do_execsql_test(
         )
     ]], {
         -- <tkt1473-5.6>
-        1
+        1, "Expression subquery returned more than 1 row"
         -- </tkt1473-5.6>
     })
 
@@ -633,7 +633,7 @@ test:do_execsql_test(
         -- </tkt1473-5.7>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "tkt1473-6.3",
     [[
         SELECT EXISTS (
@@ -659,11 +659,11 @@ test:do_execsql_test(
         )
     ]], {
         -- <tkt1473-6.3>
-        1
+        1, "Expression subquery returned more than 1 row"
         -- </tkt1473-6.3>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "tkt1473-6.4",
     [[
         SELECT EXISTS (
@@ -689,7 +689,7 @@ test:do_execsql_test(
         )
     ]], {
         -- <tkt1473-6.4>
-        1
+        1, "Expression subquery returned more than 1 row"
         -- </tkt1473-6.4>
     })
 
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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [tarantool-patches] Re: [PATCH v2 1/1] sql: disallow returning many rows from subselect
  2018-06-28 17:08       ` Kirill Shcherbatov
@ 2018-06-29 13:15         ` n.pettik
  2018-07-02  7:17           ` Kirill Shcherbatov
  0 siblings, 1 reply; 15+ messages in thread
From: n.pettik @ 2018-06-29 13:15 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Kirill Shcherbatov


> +
> +test:do_catchsql_test(
> +    "subselect-5.2",
> +    [[
> +        SELECT b from t5 WHERE a = (SELECT a FROM t5 WHERE b=6);


Please, use upper-case for all SQL keywords (here and in other SQL tests you modified).

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [tarantool-patches] Re: [PATCH v2 1/1] sql: disallow returning many rows from subselect
  2018-06-29 13:15         ` n.pettik
@ 2018-07-02  7:17           ` Kirill Shcherbatov
  2018-07-02  8:45             ` n.pettik
  0 siblings, 1 reply; 15+ messages in thread
From: Kirill Shcherbatov @ 2018-07-02  7:17 UTC (permalink / raw)
  To: tarantool-patches, n.pettik

> Please, use upper-case for all SQL keywords (here and in other SQL tests you modified).
-        CREATE TABLE t5(a int primary key, b int);
+        CREATE TABLE t5(a INT PRIMARY KEY, b INT);

-        SELECT b from t5 WHERE a = (SELECT a FROM t5 WHERE b=6);
+        SELECT b FROM t5 WHERE a = (SELECT a FROM t5 WHERE b=6);

-        SELECT b from t1 where a = (SELECT a FROM t1 WHERE b=6 LIMIT (select b-1 from t1 where a =1));
+        SELECT b FROM t1 WHERE a = (SELECT a FROM t1 WHERE b=6 LIMIT (SELECT b-1 FROM t1 WHERE a =1));


-        SELECT b from t1 where a = (SELECT a FROM t1 WHERE b=6 LIMIT (select b from t1 where a =1));
+        SELECT b FROM t1 WHERE a = (SELECT a FROM t1 WHERE b=6 LIMIT (SELECT b FROM t1 WHERE a =1));

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [tarantool-patches] Re: [PATCH v2 1/1] sql: disallow returning many rows from subselect
  2018-07-02  7:17           ` Kirill Shcherbatov
@ 2018-07-02  8:45             ` n.pettik
  2018-07-02 13:30               ` Vladislav Shpilevoy
  0 siblings, 1 reply; 15+ messages in thread
From: n.pettik @ 2018-07-02  8:45 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Kirill Shcherbatov, Vladislav Shpilevoy

Thanks, now LGTM. Vlad, could you please take a look?

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [tarantool-patches] Re: [PATCH v2 1/1] sql: disallow returning many rows from subselect
  2018-07-02  8:45             ` n.pettik
@ 2018-07-02 13:30               ` Vladislav Shpilevoy
  2018-07-02 14:14                 ` Kirill Shcherbatov
  0 siblings, 1 reply; 15+ messages in thread
From: Vladislav Shpilevoy @ 2018-07-02 13:30 UTC (permalink / raw)
  To: n.pettik, tarantool-patches; +Cc: Kirill Shcherbatov

Thanks for the patch! See 6 comments below.

And I have pushed more minor fixes on the branch. Please,
squash.

> diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
> index 70e134f21..23cee593f 100644
> --- a/src/box/sql/expr.c
> +++ b/src/box/sql/expr.c
> @@ -2874,10 +2874,14 @@ 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) {

1. How can it be != NULL?

> +				pSel->pLimit =
> +					sqlite3ExprAlloc(pParse->db, TK_INTEGER,
> +							 &sqlite3IntTokens[1],
> +							 0);
> +				ExprSetProperty(pSel->pLimit, EP_System);

2. ExprSerProperty does not check for pSel->pLimit == NULL, so it crashes on
OOM.

> +			}
> +			pSel->selFlags |= SF_SingleRow;
>  			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 54f78a9de..daec802da 100644
> --- a/src/box/sql/select.c
> +++ b/src/box/sql/select.c
> @@ -2120,6 +2120,38 @@ computeLimitRegisters(Parse * pParse, Select * p, int iBreak)
>  			sqlite3VdbeAddOp2(v, OP_IfNot, iLimit, iBreak);
>  			VdbeCoverage(v);
>  		}
> +		if (p->selFlags & SF_SingleRow) {
> +			if (ExprHasProperty(p->pLimit, EP_System)) {
> +				/*
> +				 * Indirect LIMIT 1 is allowed only for
> +				 * requests returning only 1 row.
> +				 * To test this, we change LIMIT 1 to
> +				 * LIMIT 2 and will look up LIMIT 1 overflow
> +				 * at the sqlite3Select end.
> +				 */
> +				sqlite3VdbeAddOp2(v, OP_Integer, 2, iLimit);
> +			} else {
> +				/*
> +				 * User-defined complex limit for subquery
> +				 * could be only 1 as resulting value.
> +				 */
> +				int r1 = sqlite3GetTempReg(pParse);
> +				sqlite3VdbeAddOp2(v, OP_Integer, 1, r1);
> +				int no_err = sqlite3VdbeMakeLabel(v);
> +				sqlite3VdbeAddOp3(v, OP_Eq, iLimit, no_err, r1);
> +				const char *error =
> +					"Expression subquery could be limited "
> +					"only with 1.";
> +				sqlite3VdbeAddOp4(v, OP_Halt,
> +						  SQL_TARANTOOL_ERROR,
> +						  0, 0, error, P4_STATIC);

3. I do not see where do you set an appropriate error code (p5).

> +				sqlite3VdbeResolveLabel(v, no_err);
> +				sqlite3ReleaseTempReg(pParse, r1);
> +
> +				/* Runtime checks are no longer needed. */
> +				p->selFlags &= ~SF_SingleRow;
> +			}
> +		}
> @@ -5398,6 +5430,31 @@ explain_simple_count(struct Parse *parse_context, const char *table_name)
>  	}
>  }
>  
> +/**
> + * Generate VDBE code that HALT program when subselect returned
> + * more than one row (determined as LIMIT 1 overflow).
> + * @param parser Current parsing context.
> + * @param limit_reg LIMIT register.
> + * @param end_mark mark to jump if select returned distinct one
> + *                 row as expected.
> + */
> +static void
> +vdbe_code_raise_on_multiple_rows(struct Parse *parser, int limit_reg, int end_mark)
> +{
> +	assert(limit_reg != 0);
> +	struct Vdbe *v = sqlite3GetVdbe(parser);
> +	assert(v != NULL);
> +
> +	int r1 = sqlite3GetTempReg(parser);
> +	sqlite3VdbeAddOp2(v, OP_Integer, 0, r1);
> +	sqlite3VdbeAddOp3(v, OP_Ne, r1, end_mark, limit_reg);
> +	const char *error = "Expression subquery returned more than 1 row";
> +	sqlite3VdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR,
> +			  ON_CONFLICT_ACTION_FAIL, 0,
> +			  error, P4_STATIC);

4. Same.

> +	sqlite3ReleaseTempReg(parser, r1);
> +}
> +
>  /*
>   * Generate code for the SELECT statement given in the p argument.
>   *
> @@ -6326,8 +6389,10 @@ 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. */
> +	if (p->selFlags & SF_SingleRow && p->iLimit != 0)
> +		vdbe_code_raise_on_multiple_rows(pParse, p->iLimit, iEnd);

5. Why do you have two runtime checks for the same auto limit?

> +	/* 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 e939663b6..bacf415df 100644
> --- a/src/box/sql/sqliteInt.h
> +++ b/src/box/sql/sqliteInt.h> @@ -2702,6 +2704,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 contains more than one row. */
> +#define SF_SingleRow      0x40000

6. Why not 0x20000?

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [tarantool-patches] Re: [PATCH v2 1/1] sql: disallow returning many rows from subselect
  2018-07-02 13:30               ` Vladislav Shpilevoy
@ 2018-07-02 14:14                 ` Kirill Shcherbatov
  2018-07-02 14:42                   ` Vladislav Shpilevoy
  0 siblings, 1 reply; 15+ messages in thread
From: Kirill Shcherbatov @ 2018-07-02 14:14 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Vladislav Shpilevoy

> And I have pushed more minor fixes on the branch. Please,
> squash.
Ok, tnx.

> 1. How can it be != NULL?
If user has manually wrote LIMIT expression,
like this
SELECT b FROM t1 WHERE a = (SELECT a FROM t1 WHERE b=6 LIMIT 1);
or this
SELECT b FROM t1 WHERE a = (SELECT a FROM t1 WHERE b=6 LIMIT (SELECT b-1 FROM t1 WHERE a =1));

> 2. ExprSerProperty does not check for pSel->pLimit == NULL, so it crashes on
> OOM.
-                               ExprSetProperty(pSel->pLimit, EP_System);
+                               if (pSel->pLimit != NULL) {
+                                       ExprSetProperty(pSel->pLimit,
+                                                       EP_System);
+                               }

(If pLimit is NULL sqlite3Select returns with -1 at the beginning).

> 3. I do not see where do you set an appropriate error code (p5).
> 4. Same.
@@ -2145,6 +2145,7 @@ computeLimitRegisters(Parse * pParse, Select * p, int iBreak)
                                sqlite3VdbeAddOp4(v, OP_Halt,
                                                  SQL_TARANTOOL_ERROR,
                                                  0, 0, error, P4_STATIC);
+                               sqlite3VdbeChangeP5(v, ER_SQL_EXECUTE);
                                sqlite3VdbeResolveLabel(v, no_err);
                                sqlite3ReleaseTempReg(pParse, r1);
 
@@ -5452,6 +5453,7 @@ vdbe_code_raise_on_multiple_rows(struct Parse *parser, int limit_reg, int end_ma
        sqlite3VdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR,
                          ON_CONFLICT_ACTION_FAIL, 0,
                          error, P4_STATIC);
+       sqlite3VdbeChangeP5(v, SQL_TARANTOOL_ERROR);


> 5. Why do you have two runtime checks for the same auto limit?
There are in totally different branches, with no intersection on runtime checks.
The prev. one make return few lines bellow vdbe_code_raise_on_multiple_rows.

> 6. Why not 0x20000?
No reason.
-#define SF_SingleRow      0x40000
+#define SF_SingleRow      0x20000

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [tarantool-patches] Re: [PATCH v2 1/1] sql: disallow returning many rows from subselect
  2018-07-02 14:14                 ` Kirill Shcherbatov
@ 2018-07-02 14:42                   ` Vladislav Shpilevoy
  2018-07-02 14:52                     ` Kirill Shcherbatov
  0 siblings, 1 reply; 15+ messages in thread
From: Vladislav Shpilevoy @ 2018-07-02 14:42 UTC (permalink / raw)
  To: Kirill Shcherbatov, tarantool-patches

Thanks for the fixes!
>> 3. I do not see where do you set an appropriate error code (p5).
>> 4. Same.
> @@ -2145,6 +2145,7 @@ computeLimitRegisters(Parse * pParse, Select * p, int iBreak)
>                                  sqlite3VdbeAddOp4(v, OP_Halt,
>                                                    SQL_TARANTOOL_ERROR,
>                                                    0, 0, error, P4_STATIC);
> +                               sqlite3VdbeChangeP5(v, ER_SQL_EXECUTE);
>                                  sqlite3VdbeResolveLabel(v, no_err);
>                                  sqlite3ReleaseTempReg(pParse, r1);
>   
> @@ -5452,6 +5453,7 @@ vdbe_code_raise_on_multiple_rows(struct Parse *parser, int limit_reg, int end_ma
>          sqlite3VdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR,
>                            ON_CONFLICT_ACTION_FAIL, 0,
>                            error, P4_STATIC);
> +       sqlite3VdbeChangeP5(v, SQL_TARANTOOL_ERROR);

SQL_TARANTOOL_ERROR is not error code. See errcode.h. The previous
hunk is correct.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [tarantool-patches] Re: [PATCH v2 1/1] sql: disallow returning many rows from subselect
  2018-07-02 14:42                   ` Vladislav Shpilevoy
@ 2018-07-02 14:52                     ` Kirill Shcherbatov
  2018-07-02 17:58                       ` Vladislav Shpilevoy
  0 siblings, 1 reply; 15+ messages in thread
From: Kirill Shcherbatov @ 2018-07-02 14:52 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Vladislav Shpilevoy

> SQL_TARANTOOL_ERROR is not error code. See errcode.h. The previous
> hunk is correct.
Hm, I've forgot update this piece of patch in mail.
There is no such mistake on branch already.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [tarantool-patches] Re: [PATCH v2 1/1] sql: disallow returning many rows from subselect
  2018-07-02 14:52                     ` Kirill Shcherbatov
@ 2018-07-02 17:58                       ` Vladislav Shpilevoy
  0 siblings, 0 replies; 15+ messages in thread
From: Vladislav Shpilevoy @ 2018-07-02 17:58 UTC (permalink / raw)
  To: Kirill Shcherbatov, tarantool-patches

Thanks for the fixes! LGTM.

On 02/07/2018 17:52, Kirill Shcherbatov wrote:
>> SQL_TARANTOOL_ERROR is not error code. See errcode.h. The previous
>> hunk is correct.
> Hm, I've forgot update this piece of patch in mail.
> There is no such mistake on branch already.
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [tarantool-patches] Re: [PATCH v1 1/1] sql: disallow returning many rows from subselect
  2018-06-18 11:55 [tarantool-patches] [PATCH v1 1/1] sql: disallow returning many rows from subselect Kirill Shcherbatov
  2018-06-19 17:56 ` [tarantool-patches] " n.pettik
@ 2018-07-03  8:06 ` Kirill Yukhin
  1 sibling, 0 replies; 15+ messages in thread
From: Kirill Yukhin @ 2018-07-03  8:06 UTC (permalink / raw)
  To: tarantool-patches; +Cc: v.shpilevoy, Kirill Shcherbatov

Hello,
On 18 июн 14:55, Kirill Shcherbatov wrote:
> 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.
I've checked in the patch into 2.0 branch.

--
Regards, Kirill Yukhin

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2018-07-03  8:06 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-18 11:55 [tarantool-patches] [PATCH v1 1/1] sql: disallow returning many rows from subselect Kirill Shcherbatov
2018-06-19 17:56 ` [tarantool-patches] " n.pettik
2018-06-27 15:30   ` [tarantool-patches] [PATCH v2 " Kirill Shcherbatov
2018-06-27 15:40   ` Kirill Shcherbatov
2018-06-28 13:04     ` [tarantool-patches] " n.pettik
2018-06-28 17:08       ` Kirill Shcherbatov
2018-06-29 13:15         ` n.pettik
2018-07-02  7:17           ` Kirill Shcherbatov
2018-07-02  8:45             ` n.pettik
2018-07-02 13:30               ` Vladislav Shpilevoy
2018-07-02 14:14                 ` Kirill Shcherbatov
2018-07-02 14:42                   ` Vladislav Shpilevoy
2018-07-02 14:52                     ` Kirill Shcherbatov
2018-07-02 17:58                       ` Vladislav Shpilevoy
2018-07-03  8:06 ` [tarantool-patches] Re: [PATCH v1 " Kirill Yukhin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox