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 4238030542 for ; Thu, 29 Nov 2018 09:33:17 -0500 (EST) 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 c_QzmIBdKz7V for ; Thu, 29 Nov 2018 09:33:17 -0500 (EST) Received: from smtp56.i.mail.ru (smtp56.i.mail.ru [217.69.128.36]) (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 CD5DF304F0 for ; Thu, 29 Nov 2018 09:33:15 -0500 (EST) From: Kirill Shcherbatov Subject: [tarantool-patches] [PATCH v2 1/1] sql: support HAVING without GROUP BY clause Date: Thu, 29 Nov 2018 17:33:11 +0300 Message-Id: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit 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, v.shpilevoy@tarantool.org Cc: Kirill Shcherbatov Branch: http://github.com/tarantool/tarantool/tree/kshsh/gh-2364-having-without-groupby Issue: https://github.com/tarantool/tarantool/issues/2364 Allowed to make SELECT requests that have HAVING clause without GROUP BY. It is possible when both - left and right parts of request have aggregate function or constant value. Closes #2364. @TarantoolBot document Title: HAVING without GROUP BY clause A query with a having clause should also have a group by clause. If you omit group by, all the rows not excluded by the where clause return as a single group. Because no grouping is performed between the where and having clauses, they cannot act independently of each other. Having acts like where because it affects the rows in a single group rather than groups, except the having clause can still use aggregates. Having without group by is not supported for select from multiple tables. --- src/box/sql/resolve.c | 97 +++++++++++++++++++++++--- src/box/sql/sqliteInt.h | 3 +- test/sql-tap/count.test.lua | 8 +-- test/sql-tap/select3.test.lua | 2 +- test/sql-tap/select5.test.lua | 128 +++++++++++++++++++++++++++++++++- 5 files changed, 223 insertions(+), 15 deletions(-) diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c index 9a2d6ff4e..34f0c836a 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); } @@ -1180,6 +1182,50 @@ resolveOrderGroupBy(NameContext * pNC, /* The name context of the SELECT stateme return sqlite3ResolveOrderGroupBy(pParse, pSelect, pOrderBy, zType); } +/** + * Test if specified expression is a regular literal. + * @param expr Expression to analyse. + * @retval true If expression is literal. + * @retval false Otherwise. + */ +static bool +sql_expr_is_literal(struct Expr *expr) +{ + return expr->op == TK_INTEGER || expr->op == TK_FLOAT || + expr->op == TK_BLOB || expr->op == TK_STRING || + expr->op == TK_NULL || expr->op == TK_UMINUS; +} + +/** + * Test if specified expression is a constant function. + * @param parser Parsing context. + * @param expr Expression to analyse. + * @retval true If expression is a existent constant function. + * @retval false Otherwise. + */ +static bool +sql_expr_is_constant_func(struct Parse *parser, struct Expr *expr) +{ + if (expr->op != TK_FUNCTION) + return false; + char *func_name = expr->u.zToken; + struct ExprList *args_list = expr->x.pList; + int args_count = args_list != NULL ? args_list->nExpr : 0; + struct FuncDef *func = + sqlite3FindFunction(parser->db, func_name, args_count, 0); + if (func == NULL) { + /* + * If we fail to find function with exact number + * of arguments, lets try to search similar + * function but with different number of args. + */ + func = sqlite3FindFunction(parser->db, func_name, -2, 0); + if (func == NULL) + return false; + } + return (func->funcFlags & SQLITE_FUNC_CONSTANT) != 0; +} + /* * Resolve names in the SELECT statement p and all of its descendants. */ @@ -1285,13 +1331,23 @@ 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; - /* Resolve names in the result set. */ - if (sqlite3ResolveExprListNames(&sNC, p->pEList)) - return WRC_Abort; + for (i = 0; i < p->pEList->nExpr; i++) { + struct Expr *expr = p->pEList->a[i].pExpr; + u16 has_agg_flag = sNC.ncFlags & NC_HasAgg; + sNC.ncFlags &= ~NC_HasAgg; + if (sqlite3ResolveExprNames(&sNC, expr)) + return WRC_Abort; + is_all_select_agg &= (sNC.ncFlags & NC_HasAgg) != 0 || + sql_expr_is_literal(expr) || + sql_expr_is_constant_func(pParse, + expr); + sNC.ncFlags |= has_agg_flag; + } /* 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,12 +1362,37 @@ resolveSelectStep(Walker * pWalker, Select * p) sNC.ncFlags &= ~NC_AllowAgg; } - /* If a HAVING clause is present, then there must be a GROUP BY clause. + /* + * If a HAVING clause is present, then there must + * be a GROUP BY clause or aggregate function + * should be specified. */ - if (p->pHaving && !pGroupBy) { - sqlite3ErrorMsg(pParse, - "a GROUP BY clause is required before HAVING"); - return WRC_Abort; + if (p->pHaving != NULL && pGroupBy == NULL) { + struct NameContext having_nc; + memset(&having_nc, 0, sizeof(having_nc)); + having_nc.pParse = pParse; + having_nc.ncFlags = NC_AllowAgg; + having_nc.pSrcList = p->pSrc; + if (is_all_select_agg && + sqlite3ResolveExprNames(&having_nc, + p->pHaving) != 0) + return WRC_Abort; + if ((having_nc.ncFlags & NC_HasAgg) == 0 || + (having_nc.ncFlags & NC_HasUnaggregatedId) != 0) { + const char *err_msg = + tt_sprintf("HAVING argument must " + "appear in the GROUP BY " + "clause or be used in an " + "aggregate function"); + diag_set(ClientError, ER_SQL, err_msg); + pParse->nErr++; + pParse->rc = SQL_TARANTOOL_ERROR; + return WRC_Abort; + } + sql_expr_delete(db, p->pLimit, false); + p->pLimit = + sqlite3ExprAlloc(db, TK_INTEGER, + &sqlite3IntTokens[1], 0); } /* Add the output column list to the name-context before parsing the diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h index dbf58d967..8f0e0a320 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 seen without 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 b05e3a28e..6f58210f4 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 - ]], { + ]], -- - 1, "a GROUP BY clause is required before HAVING" + {} -- - }) + ) test:do_test( "count-2.10", diff --git a/test/sql-tap/select3.test.lua b/test/sql-tap/select3.test.lua index dbc95f0d8..2704c267e 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 ]], { -- - 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" -- }) diff --git a/test/sql-tap/select5.test.lua b/test/sql-tap/select5.test.lua index 9c3cd2759..b889132aa 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( -- }) +-- +-- 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; + ]], { + -- + 1, "SQL error: HAVING argument must appear in the GROUP BY clause or be used in an aggregate function" + -- +}) + +test:do_catchsql_test( + "select5-9.2", + [[ + SELECT SUM(s1) FROM te40 HAVING s1 = 2; + ]], { + -- + 1, "SQL error: HAVING argument must appear in the GROUP BY clause or be used in an aggregate function" + -- +}) + +test:do_catchsql_test( + "select5-9.3", + [[ + SELECT s1 FROM te40 HAVING SUM(s1) = 2; + ]], { + -- + 1, "SQL error: HAVING argument must appear in the GROUP BY clause or be used in an aggregate function" + -- +}) + +test:do_execsql_test( + "select5-9.4", + [[ + SELECT SUM(s1) FROM te40 HAVING SUM(s1) > 0; + ]], { + -- + 3 + -- +}) + +test:do_execsql_test( + "select5-9.5", + [[ + SELECT MIN(s1) FROM te40 HAVING SUM(s1) > 0; + ]], { + -- + 1 + -- +}) + +test:do_execsql_test( + "select5-9.6", + [[ + SELECT SUM(s1) FROM te40 HAVING SUM(s1) < 0; + ]], + -- + {} + -- +) + +test:do_catchsql_test( + "select5-9.7", + [[ + SELECT SUM(s1),s2 FROM te40 HAVING SUM(s1) > 0; + ]], { + -- + 1, "SQL error: HAVING argument must appear in the GROUP BY clause or be used in an aggregate function" + -- +}) + +test:do_catchsql_test( + "select5-9.8", + [[ + SELECT SUM(s1) FROM te40 HAVING SUM(s1) > 0 and s2 > 0; + ]], { + -- + 1, "SQL error: HAVING argument must appear in the GROUP BY clause or be used in an aggregate function" + -- +}) + +test:do_execsql_test( + "select5-9.9", + [[ + SELECT SUM(s1) FROM te40 HAVING SUM(s1) > 0 and SUM(s2) > 0; + ]], { + -- + 3 + -- +}) + +test:do_execsql_test( + "select5-9.10", + [[ + SELECT 1 FROM te40 HAVING SUM(s1) > 0; + ]], { + -- + 1 + -- +}) + +test:do_execsql_test( + "select5-9.11", + [[ + SELECT -1 FROM te40 HAVING SUM(s1) > 0; + ]], { + -- + -1 + -- +}) + +test:do_execsql_test( + "select5-9.12", + [[ + SELECT NULL FROM te40 HAVING SUM(s1) > 0; + ]], { + -- + "" + -- +}) + test:finish_test() -- 2.19.2