Commit d2d79887 authored by Andrew Gierth's avatar Andrew Gierth

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.
parent aea7c17e
...@@ -3997,7 +3997,28 @@ consider_groupingsets_paths(PlannerInfo *root, ...@@ -3997,7 +3997,28 @@ consider_groupingsets_paths(PlannerInfo *root,
Assert(can_hash); 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); unhashed_rollup = lfirst_node(RollupData, l_start);
exclude_groups = unhashed_rollup->numGroups; exclude_groups = unhashed_rollup->numGroups;
......
...@@ -1018,6 +1018,18 @@ explain (costs off) ...@@ -1018,6 +1018,18 @@ explain (costs off)
-> Values Scan on "*VALUES*" -> Values Scan on "*VALUES*"
(9 rows) (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 -- mixed hashable/sortable cases
select unhashable_col, unsortable_col, select unhashable_col, unsortable_col,
grouping(unhashable_col, unsortable_col), grouping(unhashable_col, unsortable_col),
......
...@@ -292,6 +292,11 @@ explain (costs off) ...@@ -292,6 +292,11 @@ explain (costs off)
select a, b, grouping(a,b), array_agg(v order by v) select a, b, grouping(a,b), array_agg(v order by v)
from gstest1 group by cube(a,b); 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 -- mixed hashable/sortable cases
select unhashable_col, unsortable_col, select unhashable_col, unsortable_col,
grouping(unhashable_col, unsortable_col), grouping(unhashable_col, unsortable_col),
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment