[tarantool-patches] [PATCH v1 1/1] sql: support HAVING without GROUP BY clause

[Kirill Shcherbatov <kshcherbatov@tarantool.org>]

To allow user do not specify GROUP BY directly we build it
manually using table primary index.

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.
---
Branch: http://github.com/tarantool/tarantool/tree/kshsh/gh-2364-having-without-groupby
Issue: https://github.com/tarantool/tarantool/issues/2364

 src/box/sql/resolve.c         | 47 ++++++++++++++++++++++++++++++++++-------
 test/sql-tap/count.test.lua   |  8 +++----
 test/sql-tap/select3.test.lua |  4 ++--
 test/sql-tap/select5.test.lua | 49 ++++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 93 insertions(+), 15 deletions(-)

diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c
index 280ecd9..34bb069 100644
--- a/src/box/sql/resolve.c
+++ b/src/box/sql/resolve.c
@@ -1295,6 +1295,45 @@ resolveSelectStep(Walker * pWalker, Select * p)
 		 * expression, do not allow aggregates in any of the other expressions.
 		 */
 		assert((p->selFlags & SF_Aggregate) == 0);
+		/*
+		 * A query with a HAVING clause should also have
+		 * a group by clause. If GROUP BY is ommited, all
+		 * the rows not excluded by the where clause
+		 * return as a single group built with primary
+		 * key.
+		 */
+		if (p->pHaving != NULL && p->pGroupBy == NULL) {
+			if (p->pSrc->nSrc != 1) {
+				diag_set(ClientError, ER_SQL,
+					"GROUP BY may be ommited only when one "
+					"source table specified");
+				pParse->nErr++;
+				pParse->rc = SQL_TARANTOOL_ERROR;
+				return WRC_Abort;
+			}
+			struct space_def *def = p->pSrc->a->pTab->def;
+			struct Index *pk =
+				sqlite3PrimaryKeyIndex(p->pSrc->a->pTab);
+			assert(pk != NULL);
+			struct key_part *part = pk->def->key_def->parts;
+			struct key_part *part_end =
+				part + pk->def->key_def->part_count;
+			for (; part < part_end && !db->mallocFailed; part++) {
+				struct Token pk_column;
+				char *col_name =
+					def->fields[part->fieldno].name;
+				sqlite3TokenInit(&pk_column, col_name);
+				struct Expr *pk_part =
+					sqlite3ExprAlloc(db, TK_ID,
+							 &pk_column, 0);
+				p->pGroupBy =
+					sql_expr_list_append(db, p->pGroupBy,
+							     pk_part);
+			}
+			/* Memory allocation error. */
+			if (db->mallocFailed != 0)
+				return WRC_Abort;
+		}
 		pGroupBy = p->pGroupBy;
 		if (pGroupBy || (sNC.ncFlags & NC_HasAgg) != 0) {
 			assert(NC_MinMaxAgg == SF_MinMaxAgg);
@@ -1304,14 +1343,6 @@ 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
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..535bd5d 100755
--- a/test/sql-tap/select3.test.lua
+++ b/test/sql-tap/select3.test.lua
@@ -196,11 +196,11 @@ test:do_catchsql_test("select3-2.14", [[
 
 -- Cannot have a HAVING without a GROUP BY
 --
-test:do_catchsql_test("select3-3.1", [[
+test:do_execsql_test("select3-3.1", [[
   SELECT log, count(*) FROM t1 HAVING log>=4
 ]], {
   -- <select3-3.1>
-  1, "a GROUP BY clause is required before HAVING"
+  4,1,4,1,4,1,4,1,4,1,4,1,4,1,4,1,5,1,5,1,5,1,5,1,5,1,5,1,5,1,5,1,5,1,5,1,5,1,5,1,5,1,5,1,5,1
   -- </select3-3.1>
 })
 
diff --git a/test/sql-tap/select5.test.lua b/test/sql-tap/select5.test.lua
index 7ca25ae..353d767 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(36)
 
 --!./tcltestrunner.lua
 -- 2001 September 15
@@ -412,5 +412,52 @@ test:do_execsql_test(
         -- </select5-8.8>
     })
 
+--
+-- gh-2364: Support HAVING without GROUP BY clause.
+--
+test:do_execsql_test(
+    "select6-1.1",
+    [[
+        CREATE TABLE te40 (s1 INT, s2 INT, PRIMARY KEY (s1,s2));
+        INSERT INTO te40 VALUES (1,1);
+        SELECT s1 FROM te40 HAVING s1 = 1;
+    ]], {
+    -- <select6-1.1>
+    1
+    -- <select6-1.1>
+})
+
+test:do_execsql_test(
+    "select6-1.2",
+    [[
+        SELECT s1 FROM te40 HAVING s1 = 2;
+    ]],
+    -- <select6-1.2>
+    {}
+    -- <select6-1.2>
+)
+
+test:do_catchsql_test(
+    "select6-2.1",
+    [[
+        CREATE TABLE te4 (s1 INT PRIMARY KEY);
+        INSERT INTO te4 VALUES (1);
+        SELECT te40.s1 FROM te40,te4 HAVING s1 = 1;
+    ]], {
+    -- <select6-2.1>
+    1, "SQL error: GROUP BY may be ommited only when one source table specified"
+    -- <select6-2.1>
+})
+
+test:do_execsql_test(
+    "select6-3.1",
+    [[
+        SELECT SUM(s1) FROM te40 HAVING SUM(s1) > 0;
+    ]], {
+    -- <select6-3.1>
+    1
+    -- <select6-3.1>
+})
+
 test:finish_test()
 
-- 
2.7.4