diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 879220c63e498f423b53c4f57652efb9cea07cd7..dc9053bca59e10bfd7ceffaeb3a56d122e138b35 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -3440,7 +3440,6 @@ static List * reorder_grouping_sets(List *groupingsets, List *sortclause) { ListCell *lc; - ListCell *lc2; List *previous = NIL; List *result = NIL; @@ -3449,34 +3448,32 @@ reorder_grouping_sets(List *groupingsets, List *sortclause) List *candidate = lfirst(lc); List *new_elems = list_difference_int(candidate, previous); - if (list_length(new_elems) > 0) + while (list_length(sortclause) > list_length(previous) && + list_length(new_elems) > 0) { - while (list_length(sortclause) > list_length(previous)) - { - SortGroupClause *sc = list_nth(sortclause, list_length(previous)); - int ref = sc->tleSortGroupRef; + SortGroupClause *sc = list_nth(sortclause, list_length(previous)); + int ref = sc->tleSortGroupRef; - if (list_member_int(new_elems, ref)) - { - previous = lappend_int(previous, ref); - new_elems = list_delete_int(new_elems, ref); - } - else - { - /* diverged from the sortclause; give up on it */ - sortclause = NIL; - break; - } + if (list_member_int(new_elems, ref)) + { + previous = lappend_int(previous, ref); + new_elems = list_delete_int(new_elems, ref); } - - foreach(lc2, new_elems) + else { - previous = lappend_int(previous, lfirst_int(lc2)); + /* diverged from the sortclause; give up on it */ + sortclause = NIL; + break; } } + /* + * Safe to use list_concat (which shares cells of the second arg) + * because we know that new_elems does not share cells with anything. + */ + previous = list_concat(previous, new_elems); + result = lcons(list_copy(previous), result); - list_free(new_elems); } list_free(previous); diff --git a/src/test/regress/expected/groupingsets.out b/src/test/regress/expected/groupingsets.out index 30e780f0b8bde3cca5acf3ee75a7d559646d7756..1843d2d957886af8412ab8d001bb574c3a6a3161 100644 --- a/src/test/regress/expected/groupingsets.out +++ b/src/test/regress/expected/groupingsets.out @@ -600,6 +600,19 @@ select a, b, sum(v.x) | | 9 (12 rows) +-- Test reordering of grouping sets +explain (costs off) +select * from gstest1 group by grouping sets((a,b,v),(v)) order by v,b,a; + QUERY PLAN +------------------------------------------------------------------------------ + GroupAggregate + Group Key: "*VALUES*".column3, "*VALUES*".column2, "*VALUES*".column1 + Group Key: "*VALUES*".column3 + -> Sort + Sort Key: "*VALUES*".column3, "*VALUES*".column2, "*VALUES*".column1 + -> Values Scan on "*VALUES*" +(6 rows) + -- Agg level check. This query should error out. select (select grouping(a,b) from gstest2) from gstest2 group by a,b; ERROR: arguments to GROUPING must be grouping expressions of the associated query level diff --git a/src/test/regress/sql/groupingsets.sql b/src/test/regress/sql/groupingsets.sql index ce920b79e410f22163d6ac331b95d74c6ab11b21..1a588f415c91eff773da34adcc126e9869c272e8 100644 --- a/src/test/regress/sql/groupingsets.sql +++ b/src/test/regress/sql/groupingsets.sql @@ -191,6 +191,9 @@ select a, b, sum(v.x) from (values (1),(2)) v(x), gstest_data(v.x) group by cube (a,b) order by a,b; +-- Test reordering of grouping sets +explain (costs off) +select * from gstest1 group by grouping sets((a,b,v),(v)) order by v,b,a; -- Agg level check. This query should error out. select (select grouping(a,b) from gstest2) from gstest2 group by a,b;