From: Kirill Shcherbatov <kshcherbatov@tarantool.org> To: tarantool-patches@freelists.org, Nikita Pettik <korablev@tarantool.org> Subject: [tarantool-patches] Re: [PATCH v1 1/1] sql: support HAVING without GROUP BY clause Date: Mon, 10 Sep 2018 10:51:24 +0300 [thread overview] Message-ID: <077750b5-7433-6122-5df9-caa6ff462e70@tarantool.org> (raw) In-Reply-To: <99A1FBB4-3831-4BB6-A1D6-505D09F792AD@tarantool.org> > db2 => SELECT SUM(a), a from t1 having sum(a) > 0 > SQL0119N An expression starting with "A" specified in a SELECT clause, HAVING > clause, or ORDER BY clause is not specified in the GROUP BY clause or it is in > a SELECT clause, HAVING clause, or ORDER BY clause with a column function and > no GROUP BY clause is specified. SQLSTATE=42803 I am able to handle this for now. > I would re-phrase it like: > > However, it is still possible to drop GROUP BY clause in case both SELECT > and HAVING arguments are aggregate functions Ok, tnx. > Nit: HAVING is rather clause, than expression. Overall, this msg looks > a little bit weird. In Postgres it is like: > > column "t1.a" must appear in the GROUP BY clause or be used in an aggregate function > > So, not HAVING clause must appear, but HAVING argument. ---> HAVING argument > Why 2.1 and not 1.5? The same for tests below. It was totall invalid naming. I've fixed it. ============================================================== From fd960870f3519d335ee5dc61fbdd950d297fe79c Mon Sep 17 00:00:00 2001 Message-Id: <fd960870f3519d335ee5dc61fbdd950d297fe79c.1536565618.git.kshcherbatov@tarantool.org> From: Kirill Shcherbatov <kshcherbatov@tarantool.org> Date: Wed, 29 Aug 2018 16:24:09 +0300 Subject: [PATCH] sql: support HAVING without GROUP BY Allowed to use HAVING without GROUP BY when both select list and having clause have aggregate function. Closes 2364. @TarantoolBot document Title: HAVING without GROUP BY clause A query with a HAVING clause should also have a GROUP BY clause. Another ability is to use aggregate function both in select list and HAVING clause. Example: SELECT SUM(s1) FROM te40 HAVING SUM(s1) > 0; --- src/box/sql/resolve.c | 45 ++++++++++++++++---- 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 | 98 ++++++++++++++++++++++++++++++++++++++++++- 5 files changed, 141 insertions(+), 15 deletions(-) diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c index 280ecd9..c38374d 100644 --- a/src/box/sql/resolve.c +++ b/src/box/sql/resolve.c @@ -600,6 +600,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); } @@ -1283,13 +1285,19 @@ resolveSelectStep(Walker * pWalker, Select * p) /* Set up the local name-context to pass to sqlite3ResolveExprNames() to * resolve the result-set expression list. */ + bool 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++) { + u16 has_agg_flag = sNC.ncFlags & NC_HasAgg; + sNC.ncFlags &= ~NC_HasAgg; + if (sqlite3ResolveExprNames(&sNC, p->pEList->a[i].pExpr)) + return WRC_Abort; + all_select_agg &= (sNC.ncFlags & NC_HasAgg) != 0; + 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. @@ -1304,12 +1312,33 @@ 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 (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; + } } /* 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 abb6829..6c08fd1 100644 --- a/src/box/sql/sqliteInt.h +++ b/src/box/sql/sqliteInt.h @@ -2528,7 +2528,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 45808de..700d047 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 d49bb87..c2e901e 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 7ca25ae..d5bab1d 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(41) --!./tcltestrunner.lua -- 2001 September 15 @@ -412,5 +412,101 @@ 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:finish_test() -- 2.7.4
next prev parent reply other threads:[~2018-09-10 7:51 UTC|newest] Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-08-28 16:39 [tarantool-patches] " Kirill Shcherbatov 2018-08-29 11:52 ` [tarantool-patches] " n.pettik 2018-08-29 13:29 ` Kirill Shcherbatov 2018-09-03 11:24 ` n.pettik 2018-09-03 13:23 ` n.pettik 2018-09-10 7:51 ` Kirill Shcherbatov [this message] 2018-09-10 21:43 ` n.pettik 2018-09-20 12:43 ` Kirill Shcherbatov 2018-10-01 16:37 ` n.pettik
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=077750b5-7433-6122-5df9-caa6ff462e70@tarantool.org \ --to=kshcherbatov@tarantool.org \ --cc=korablev@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='[tarantool-patches] Re: [PATCH v1 1/1] sql: support HAVING without GROUP BY clause' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox