From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 552A424D43 for ; Mon, 18 Jun 2018 07:55:27 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id J9rn19wxCrP9 for ; Mon, 18 Jun 2018 07:55:27 -0400 (EDT) Received: from smtp16.mail.ru (smtp16.mail.ru [94.100.176.153]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 6E93C249A0 for ; Mon, 18 Jun 2018 07:55:26 -0400 (EDT) From: Kirill Shcherbatov Subject: [tarantool-patches] [PATCH v1 1/1] sql: disallow returning many rows from subselect Date: Mon, 18 Jun 2018 14:55:03 +0300 Message-Id: Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: v.shpilevoy@tarantool.org, 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; ]], { -- 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; ]], { -- - "two" + "one" -- }) 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)) ]], { -- "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); ]], { -- 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; ]], { -- 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 ]], { -- 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 ]], { -- 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 ]], { -- 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 ]], { -- 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 ]], { -- 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)); ]], { -- 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)); ]], { -- 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) ]], { -- 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); ]], { -- 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); ]], { -- 6 @@ -338,6 +338,30 @@ test:do_execsql_test( -- }) +-- 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); + ]], { + -- + 1, "SELECT subquery returned more than 1 row" + -- +}) +test:do_catchsql_test( + "subselect-5.2", + [[ + SELECT b from t5 WHERE a = (SELECT a FROM t5 WHERE b=6); + ]], { + -- + 1, "SELECT subquery returned more than 1 row" + -- +}) 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