1
0
mirror of https://github.com/postgres/postgres.git synced 2025-07-28 23:42:10 +03:00

Repair logic for reordering grouping sets optimization.

The logic in reorder_grouping_sets to order grouping set elements to
match a pre-specified sort ordering was defective, resulting in
unnecessary sort nodes (though the query output would still be
correct). Repair, simplifying the code a little, and add a test.

Per report from Richard Guo, though I didn't use their patch. Original
bug seems to have been my fault.

Backpatch back to 9.5 where grouping sets were introduced.

Discussion: https://postgr.es/m/CAN_9JTzyjGcUjiBHxLsgqfk7PkdLGXiM=pwM+=ph2LsWw0WO1A@mail.gmail.com
This commit is contained in:
Andrew Gierth
2019-06-30 23:49:13 +01:00
parent c000a47ad1
commit da53be23d1
3 changed files with 34 additions and 21 deletions

View File

@ -3530,7 +3530,6 @@ static List *
reorder_grouping_sets(List *groupingsets, List *sortclause)
{
ListCell *lc;
ListCell *lc2;
List *previous = NIL;
List *result = NIL;
@ -3540,35 +3539,33 @@ reorder_grouping_sets(List *groupingsets, List *sortclause)
List *new_elems = list_difference_int(candidate, previous);
GroupingSetData *gs = makeNode(GroupingSetData);
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);
gs->set = list_copy(previous);
result = lcons(gs, result);
list_free(new_elems);
}
list_free(previous);