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.

diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 4dae405bd5625792cc3bbc4cb7783d916e73ce93..1946f9ef968412821ab74d28a508fca78d9eeaf4 100644 (file)
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -4200,7 +4200,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(l_start);
                        exclude_groups = unhashed_rollup->numGroups;

diff --git a/src/test/regress/expected/groupingsets.out b/src/test/regress/expected/groupingsets.out
index d21a494a9ddfc2d2243154ce6bfc741e887d858b..c7deec2ff402667fc2a21e990e4cabddf78c41bb 100644 (file)
--- 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 eb680286030d6d952bc7e88c891503dc54ee1a22..c32d23b8d72d9759469c25bf57509c65f5d1a29e 100644 (file)
--- 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),