Hi,
I have observed some fishy behavior related to GROUPING in HAVING clause
and when we have only one element in GROUPING SETS.
Basically, when we have only one element in GROUING SETS, we are assuming
it as a simple GROUP BY with one column. Due to which we are ending up with
this error.
If we have ROLLUP/CUBE or GROUPING SETS with multiple elements, then we are
not getting this error.
Here are some examples:
postgres=# select ten, grouping(ten) from onek
postgres-# group by grouping sets(ten) having grouping(ten) >= 0
postgres-# order by 2,1;
ERROR: parent of GROUPING is not Agg node
postgres=# select ten, grouping(ten) from onek
postgres-# group by grouping sets(ten, four) having grouping(ten) > 0
postgres-# order by 2,1;
ten | grouping
-----+----------
(0 rows)fix_bug_related_to_grouping_v1.patch
postgres=# select ten, grouping(ten) from onek
postgres-# group by rollup(ten) having grouping(ten) > 0
postgres-# order by 2,1;
ten | grouping
-----+----------
| 1
(1 row)
postgres=# select ten, grouping(ten) from onek
postgres-# group by cube(ten) having grouping(ten) > 0
postgres-# order by 2,1;
ten | grouping
-----+----------
| 1
(1 row)
I had a look over relevant code and found that contain_agg_clause_walker()
is not considering GroupingFunc as an aggregate node, due to which it is
failing to consider it in a having clause in subquery_planner().
Fix this by adding GroupingFunc node in this walker. We do it correctly in
contain_aggs_of_level_walker() in which we have handling for GroupingFunc
there.
Attached patch to fix this.
The side effect is that, if we have plain group by clause, then too we can
use GROUPING in HAVING clause. But I guess it is fine.
Let me know if I missed anything to consider here.
Thanks
--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index d40083d..cdb6955 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -390,7 +390,7 @@ make_ands_implicit(Expr *clause)
/*
* contain_agg_clause
- * Recursively search for Aggref nodes within a clause.
+ * Recursively search for Aggref/GroupingFunc nodes within a clause.
*
* Returns true if any aggregate found.
*
@@ -417,6 +417,11 @@ contain_agg_clause_walker(Node *node, void *context)
Assert(((Aggref *) node)->agglevelsup == 0);
return true; /* abort the tree traversal and return true */
}
+ if (IsA(node, GroupingFunc))
+ {
+ Assert(((GroupingFunc *) node)->agglevelsup == 0);
+ return true; /* abort the tree traversal and return true */
+ }
Assert(!IsA(node, SubLink));
return expression_tree_walker(node, contain_agg_clause_walker, context);
}
diff --git a/src/test/regress/expected/groupingsets.out b/src/test/regress/expected/groupingsets.out
index 842c2ae..adb39b3 100644
--- a/src/test/regress/expected/groupingsets.out
+++ b/src/test/regress/expected/groupingsets.out
@@ -486,6 +486,68 @@ having exists (select 1 from onek b where sum(distinct a.four) = b.four);
9 | 3
(25 rows)
+-- HAVING with GROUPING queries
+select ten, grouping(ten) from onek
+group by grouping sets(ten) having grouping(ten) >= 0
+order by 2,1;
+ ten | grouping
+-----+----------
+ 0 | 0
+ 1 | 0
+ 2 | 0
+ 3 | 0
+ 4 | 0
+ 5 | 0
+ 6 | 0
+ 7 | 0
+ 8 | 0
+ 9 | 0
+(10 rows)
+
+select ten, grouping(ten) from onek
+group by grouping sets(ten, four) having grouping(ten) > 0
+order by 2,1;
+ ten | grouping
+-----+----------
+ | 1
+ | 1
+ | 1
+ | 1
+(4 rows)
+
+select ten, grouping(ten) from onek
+group by rollup(ten) having grouping(ten) > 0
+order by 2,1;
+ ten | grouping
+-----+----------
+ | 1
+(1 row)
+
+select ten, grouping(ten) from onek
+group by cube(ten) having grouping(ten) > 0
+order by 2,1;
+ ten | grouping
+-----+----------
+ | 1
+(1 row)
+
+select ten, grouping(ten) from onek
+group by (ten) having grouping(ten) >= 0
+order by 2,1;
+ ten | grouping
+-----+----------
+ 0 | 0
+ 1 | 0
+ 2 | 0
+ 3 | 0
+ 4 | 0
+ 5 | 0
+ 6 | 0
+ 7 | 0
+ 8 | 0
+ 9 | 0
+(10 rows)
+
-- FILTER queries
select ten, sum(distinct four) filter (where four::text ~ '123') from onek a
group by rollup(ten);
diff --git a/src/test/regress/sql/groupingsets.sql b/src/test/regress/sql/groupingsets.sql
index 0bffb85..0883afd 100644
--- a/src/test/regress/sql/groupingsets.sql
+++ b/src/test/regress/sql/groupingsets.sql
@@ -154,6 +154,23 @@ select ten, sum(distinct four) from onek a
group by grouping sets((ten,four),(ten))
having exists (select 1 from onek b where sum(distinct a.four) = b.four);
+-- HAVING with GROUPING queries
+select ten, grouping(ten) from onek
+group by grouping sets(ten) having grouping(ten) >= 0
+order by 2,1;
+select ten, grouping(ten) from onek
+group by grouping sets(ten, four) having grouping(ten) > 0
+order by 2,1;
+select ten, grouping(ten) from onek
+group by rollup(ten) having grouping(ten) > 0
+order by 2,1;
+select ten, grouping(ten) from onek
+group by cube(ten) having grouping(ten) > 0
+order by 2,1;
+select ten, grouping(ten) from onek
+group by (ten) having grouping(ten) >= 0
+order by 2,1;
+
-- FILTER queries
select ten, sum(distinct four) filter (where four::text ~ '123') from onek a
group by rollup(ten);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
