diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c index 9a2d6ff4e599420b37ddf7d402eb26b15d4ae79e..6462467bc4d5bd6e55d892611a2f50eef56a4226 100644 --- a/src/box/sql/resolve.c +++ b/src/box/sql/resolve.c @@ -602,6 +602,8 @@ resolveExprStep(Walker * pWalker, Expr * pExpr) /* A lone identifier is the name of a column. */ case TK_ID:{ + if ((pNC->ncFlags & NC_AllowAgg) != 0) + pNC->ncFlags |= NC_HasUnaggregatedId; return lookupName(pParse, 0, pExpr->u.zToken, pNC, pExpr); } @@ -1285,13 +1287,34 @@ resolveSelectStep(Walker * pWalker, Select * p) /* Set up the local name-context to pass to sqlite3ResolveExprNames() to * resolve the result-set expression list. */ + bool is_all_select_agg = true; sNC.ncFlags = NC_AllowAgg; sNC.pSrcList = p->pSrc; sNC.pNext = pOuterNC; - + struct ExprList_item *item = p->pEList->a; /* Resolve names in the result set. */ - if (sqlite3ResolveExprListNames(&sNC, p->pEList)) - return WRC_Abort; + for (i = 0; i < p->pEList->nExpr; ++i, ++item) { + u16 has_agg_flag = sNC.ncFlags & NC_HasAgg; + sNC.ncFlags &= ~NC_HasAgg; + if (sqlite3ResolveExprNames(&sNC, item->pExpr) != 0) + return WRC_Abort; + if ((sNC.ncFlags & NC_HasAgg) == 0 && + !sqlite3ExprIsConstantOrFunction(item->pExpr, 0)) { + is_all_select_agg = false; + sNC.ncFlags |= has_agg_flag; + break; + } + sNC.ncFlags |= has_agg_flag; + } + /* + * Finish iteration for is_all_select_agg == false + * and do not care about flags anymore. + */ + for (; i < p->pEList->nExpr; ++i, ++item) { + assert(! is_all_select_agg); + if (sqlite3ResolveExprNames(&sNC, item->pExpr) != 0) + return WRC_Abort; + } /* If there are no aggregate functions in the result-set, and no GROUP BY * expression, do not allow aggregates in any of the other expressions. @@ -1306,25 +1329,53 @@ resolveSelectStep(Walker * pWalker, Select * p) sNC.ncFlags &= ~NC_AllowAgg; } - /* If a HAVING clause is present, then there must be a GROUP BY clause. - */ - if (p->pHaving && !pGroupBy) { - sqlite3ErrorMsg(pParse, - "a GROUP BY clause is required before HAVING"); - return WRC_Abort; - } - - /* Add the output column list to the name-context before parsing the - * other expressions in the SELECT statement. This is so that - * expressions in the WHERE clause (etc.) can refer to expressions by - * aliases in the result set. + /* + * Add the output column list to the name-context + * before parsing the other expressions in the + * SELECT statement. This is so that expressions + * in the WHERE clause (etc.) can refer to + * expressions by aliases in the result set. * - * Minor point: If this is the case, then the expression will be - * re-evaluated for each reference to it. + * Minor point: If this is the case, then the + * expression will be re-evaluated for each + * reference to it. */ sNC.pEList = p->pEList; - if (sqlite3ResolveExprNames(&sNC, p->pHaving)) - return WRC_Abort; + /* + * If a HAVING clause is present, then there must + * be a GROUP BY clause or aggregate function + * should be specified. + */ + if (p->pHaving != NULL && pGroupBy == NULL) { + sNC.ncFlags |= NC_AllowAgg; + if (is_all_select_agg && + sqlite3ResolveExprNames(&sNC, p->pHaving) != 0) + return WRC_Abort; + if ((sNC.ncFlags & NC_HasAgg) == 0 || + (sNC.ncFlags & NC_HasUnaggregatedId) != 0) { + diag_set(ClientError, ER_SQL, "HAVING " + "argument must appear in the GROUP BY " + "clause or be used in an aggregate " + "function"); + pParse->nErr++; + pParse->rc = SQL_TARANTOOL_ERROR; + return WRC_Abort; + } + /* + * Aggregate functions may return only + * one tuple, so user-defined LIMITs have + * no sense (most DBs don't support such + * LIMIT but there is no reason to + * restrict it directly). + */ + sql_expr_delete(db, p->pLimit, false); + p->pLimit = + sqlite3ExprAlloc(db, TK_INTEGER, + &sqlite3IntTokens[1], 0); + } else { + if (sqlite3ResolveExprNames(&sNC, p->pHaving)) + return WRC_Abort; + } if (sqlite3ResolveExprNames(&sNC, p->pWhere)) return WRC_Abort; diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h index 4110a599116114699bed8e252a168323f909a129..7501fadc8026df03eb0667f64f9ada17dbe136d3 100644 --- a/src/box/sql/sqliteInt.h +++ b/src/box/sql/sqliteInt.h @@ -2454,7 +2454,8 @@ struct NameContext { #define NC_IdxExpr 0x0020 /* True if resolving columns of CREATE INDEX */ #define NC_VarSelect 0x0040 /* A correlated subquery has been seen */ #define NC_MinMaxAgg 0x1000 /* min/max aggregates seen. See note above */ - +/** One or more identifiers are out of aggregate function. */ +#define NC_HasUnaggregatedId 0x2000 /* * An instance of the following structure contains all information * needed to generate code for a single SELECT statement. diff --git a/test/sql-tap/count.test.lua b/test/sql-tap/count.test.lua index b05e3a28e0966b0d6089be1e4a0701cdb02b433e..6f58210f481e3ce5e3f6c99e85714201ec1deda9 100755 --- a/test/sql-tap/count.test.lua +++ b/test/sql-tap/count.test.lua @@ -172,15 +172,15 @@ test:do_test( return uses_op_count("SELECT count(*) FROM t2 WHERE a IS NOT NULL") end, 0) -test:do_catchsql_test( +test:do_execsql_test( "count-2.9", [[ SELECT count(*) FROM t2 HAVING count(*)>1 - ]], { + ]], -- <count-2.9> - 1, "a GROUP BY clause is required before HAVING" + {} -- </count-2.9> - }) + ) test:do_test( "count-2.10", diff --git a/test/sql-tap/select3.test.lua b/test/sql-tap/select3.test.lua index dbc95f0d8e22669ef3b4f0b9b073cbc34db16cd2..2704c267ebe6261f5a9bc391032948778d920052 100755 --- a/test/sql-tap/select3.test.lua +++ b/test/sql-tap/select3.test.lua @@ -200,7 +200,7 @@ test:do_catchsql_test("select3-3.1", [[ SELECT log, count(*) FROM t1 HAVING log>=4 ]], { -- <select3-3.1> - 1, "a GROUP BY clause is required before HAVING" + 1, "SQL error: HAVING argument must appear in the GROUP BY clause or be used in an aggregate function" -- </select3-3.1> }) diff --git a/test/sql-tap/select5.test.lua b/test/sql-tap/select5.test.lua index 9c3cd27597600140f37e54dc69127421134310e9..b889132aa38bd5f4f807356acae4041632b7adde 100755 --- a/test/sql-tap/select5.test.lua +++ b/test/sql-tap/select5.test.lua @@ -1,6 +1,6 @@ #!/usr/bin/env tarantool test = require("sqltester") -test:plan(32) +test:plan(44) --!./tcltestrunner.lua -- 2001 September 15 @@ -412,5 +412,131 @@ test:do_execsql_test( -- </select5-8.8> }) +-- +-- gh-2364: Support HAVING without GROUP BY clause. +-- +test:do_catchsql_test( + "select5-9.1", + [[ + CREATE TABLE te40 (s1 INT, s2 INT, PRIMARY KEY (s1,s2)); + INSERT INTO te40 VALUES (1,1); + INSERT INTO te40 VALUES (2,2); + SELECT s1 FROM te40 HAVING s1 = 1; + ]], { + -- <select5-9.1> + 1, "SQL error: HAVING argument must appear in the GROUP BY clause or be used in an aggregate function" + -- </select5-9.1> +}) + +test:do_catchsql_test( + "select5-9.2", + [[ + SELECT SUM(s1) FROM te40 HAVING s1 = 2; + ]], { + -- <select5-9.2> + 1, "SQL error: HAVING argument must appear in the GROUP BY clause or be used in an aggregate function" + -- </select5-9.2> +}) + +test:do_catchsql_test( + "select5-9.3", + [[ + SELECT s1 FROM te40 HAVING SUM(s1) = 2; + ]], { + -- <select5-9.3> + 1, "SQL error: HAVING argument must appear in the GROUP BY clause or be used in an aggregate function" + -- </select5-9.3> +}) + +test:do_execsql_test( + "select5-9.4", + [[ + SELECT SUM(s1) FROM te40 HAVING SUM(s1) > 0; + ]], { + -- <select5-9.4> + 3 + -- </select5-9.4> +}) + +test:do_execsql_test( + "select5-9.5", + [[ + SELECT MIN(s1) FROM te40 HAVING SUM(s1) > 0; + ]], { + -- <select5-9.5> + 1 + -- </select5-9.5> +}) + +test:do_execsql_test( + "select5-9.6", + [[ + SELECT SUM(s1) FROM te40 HAVING SUM(s1) < 0; + ]], + -- <select5-9.6> + {} + -- </select5-9.6> +) + +test:do_catchsql_test( + "select5-9.7", + [[ + SELECT SUM(s1),s2 FROM te40 HAVING SUM(s1) > 0; + ]], { + -- <select5-9.7> + 1, "SQL error: HAVING argument must appear in the GROUP BY clause or be used in an aggregate function" + -- </select5-9.7> +}) + +test:do_catchsql_test( + "select5-9.8", + [[ + SELECT SUM(s1) FROM te40 HAVING SUM(s1) > 0 and s2 > 0; + ]], { + -- <select5-9.8> + 1, "SQL error: HAVING argument must appear in the GROUP BY clause or be used in an aggregate function" + -- </select5-9.8> +}) + +test:do_execsql_test( + "select5-9.9", + [[ + SELECT SUM(s1) FROM te40 HAVING SUM(s1) > 0 and SUM(s2) > 0; + ]], { + -- <select5-9.9> + 3 + -- </select5-9.9> +}) + +test:do_execsql_test( + "select5-9.10", + [[ + SELECT 1 FROM te40 HAVING SUM(s1) > 0; + ]], { + -- <select5-9.10> + 1 + -- </select5-9.10> +}) + +test:do_execsql_test( + "select5-9.11", + [[ + SELECT -1 FROM te40 HAVING SUM(s1) > 0; + ]], { + -- <select5-9.11> + -1 + -- </select5-9.11> +}) + +test:do_execsql_test( + "select5-9.12", + [[ + SELECT NULL FROM te40 HAVING SUM(s1) > 0; + ]], { + -- <select5-9.12> + "" + -- </select5-9.12> +}) + test:finish_test()