From d2d79887eadff72c339a072ef693bb6016651d30 Mon Sep 17 00:00:00 2001
From: Andrew Gierth <rhodiumtoad@postgresql.org>
Date: Wed, 21 Mar 2018 10:42:04 +0000
Subject: [PATCH] Repair crash with unsortable grouping sets.
If there were multiple grouping sets, none of them empty, all of which
were unsortable, then an oversight in consider_groupingsets_paths led
to a null pointer dereference. Fix, and add a regression test for this
case.
Per report from Dang Minh Huong, though I didn't use their patch.
Backpatch to 10.x where hashed grouping sets were added.
---
src/backend/optimizer/plan/planner.c | 23 +++++++++++++++++++++-
src/test/regress/expected/groupingsets.out | 12 +++++++++++
src/test/regress/sql/groupingsets.sql | 5 +++++
3 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index b452da0204..85805ff5c7 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -3997,7 +3997,28 @@ consider_groupingsets_paths(PlannerInfo *root,
Assert(can_hash);
- if (pathkeys_contained_in(root->group_pathkeys, path->pathkeys))
+ /*
+ * If the input is coincidentally sorted usefully (which can happen
+ * even if is_sorted is false, since that only means that our caller
+ * has set up the sorting for us), then save some hashtable space by
+ * making use of that. But we need to watch out for degenerate cases:
+ *
+ * 1) If there are any empty grouping sets, then group_pathkeys might
+ * be NIL if all non-empty grouping sets are unsortable. In this case,
+ * there will be a rollup containing only empty groups, and the
+ * pathkeys_contained_in test is vacuously true; this is ok.
+ *
+ * XXX: the above relies on the fact that group_pathkeys is generated
+ * from the first rollup. If we add the ability to consider multiple
+ * sort orders for grouping input, this assumption might fail.
+ *
+ * 2) If there are no empty sets and only unsortable sets, then the
+ * rollups list will be empty (and thus l_start == NULL), and
+ * group_pathkeys will be NIL; we must ensure that the vacuously-true
+ * pathkeys_contain_in test doesn't cause us to crash.
+ */
+ if (l_start != NULL &&
+ pathkeys_contained_in(root->group_pathkeys, path->pathkeys))
{
unhashed_rollup = lfirst_node(RollupData, l_start);
exclude_groups = unhashed_rollup->numGroups;
diff --git a/src/test/regress/expected/groupingsets.out b/src/test/regress/expected/groupingsets.out
index d21a494a9d..c7deec2ff4 100644
--- a/src/test/regress/expected/groupingsets.out
+++ b/src/test/regress/expected/groupingsets.out
@@ -1018,6 +1018,18 @@ explain (costs off)
-> Values Scan on "*VALUES*"
(9 rows)
+-- unsortable cases
+select unsortable_col, count(*)
+ from gstest4 group by grouping sets ((unsortable_col),(unsortable_col))
+ order by unsortable_col::text;
+ unsortable_col | count
+----------------+-------
+ 1 | 4
+ 1 | 4
+ 2 | 4
+ 2 | 4
+(4 rows)
+
-- mixed hashable/sortable cases
select unhashable_col, unsortable_col,
grouping(unhashable_col, unsortable_col),
diff --git a/src/test/regress/sql/groupingsets.sql b/src/test/regress/sql/groupingsets.sql
index eb68028603..c32d23b8d7 100644
--- a/src/test/regress/sql/groupingsets.sql
+++ b/src/test/regress/sql/groupingsets.sql
@@ -292,6 +292,11 @@ explain (costs off)
select a, b, grouping(a,b), array_agg(v order by v)
from gstest1 group by cube(a,b);
+-- unsortable cases
+select unsortable_col, count(*)
+ from gstest4 group by grouping sets ((unsortable_col),(unsortable_col))
+ order by unsortable_col::text;
+
-- mixed hashable/sortable cases
select unhashable_col, unsortable_col,
grouping(unhashable_col, unsortable_col),