Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ADBDEV-6314: Fix unexpected output for empty grouping set #1117

Open
wants to merge 6 commits into
base: adb-6.x-dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions src/backend/nodes/list.c
Original file line number Diff line number Diff line change
Expand Up @@ -837,6 +837,32 @@ list_intersection(const List *list1, const List *list2)
return result;
}

/*
* As list_intersection but operates on lists of integers.
*/
List *
list_intersection_int(const List *list1, const List *list2)
{
List *result;
const ListCell *cell;

if (list1 == NIL || list2 == NIL)
return NIL;

Assert(IsIntegerList(list1));
Assert(IsIntegerList(list2));

result = NIL;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

List *result = NIL;

foreach(cell, list1)
{
if (list_member_int(list2, lfirst_int(cell)))
result = lappend_int(result, lfirst_int(cell));
}

check_list_invariants(result);
return result;
}

/*
* Return a list that contains all the cells in list1 that are not in
* list2. The returned list is freshly allocated via palloc(), but the
Expand Down
28 changes: 8 additions & 20 deletions src/backend/parser/parse_agg.c
Original file line number Diff line number Diff line change
Expand Up @@ -878,11 +878,12 @@ parseCheckAggregates(ParseState *pstate, Query *qry)
}

/*
* Getting adjacent TargetEntrys in groupings. Common tle will be needed
* in case of ungrouped attributes in target list. In fact we get a list
* of tle->ressortgroupref because we need to check for matching expressions
* after flatten_joinalias_vars. We also find out if there are grouping
* expressions.
* Getting ressortgrouprefs of common TargetEntrys in groupings.
* TargetEntry is common if it appears in several grouping expressions

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"is common if it appears in several grouping expressions
* (including nesting) and in a regular GROUP BY (general case)."

Not if

"is common if it appears in all grouping expressions
* (including nesting) or in a regular GROUP BY (general case)."

?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then for the case of GROUPING SETS(a) I will point out that it degenerates into a regular GROUP BY.

* (including nesting) and in a regular GROUP BY (general case). They
* will be needed in case of ungrouped attributes in target list. We get
* a list of tle->ressortgroupref instead of tle because we need to check
* for matching expressions after flatten_joinalias_vars.
*/
comGroupingRessortgrouprefs = get_com_grouping_ressortgroupref(qry, groupClauses);

Expand Down Expand Up @@ -1591,20 +1592,7 @@ get_com_grouping_ressortgroupref_routine(Node *grpcl, List *targetList, List *co
return NIL;

/*Exclude refs that did not appear in this grouping*/
ListCell *lc = list_head(com_refs);
while (lc)
{
if (list_member_int(grouping_refs, lfirst_int(lc)))
{
lc = lnext(lc);
}
else
{
ListCell *lc_del = lc;
lc = lnext(lc);
com_refs = list_delete_int(com_refs, lfirst_int(lc_del));
}
}
com_refs = list_intersection_int(com_refs, grouping_refs);
}

return com_refs;
Expand Down Expand Up @@ -1661,7 +1649,7 @@ get_com_grouping_ressortgroupref(Query *qry, List *grp_tles)
list_append_unique_int(com_group_expr_ressortgroupref, te->ressortgroupref);
}
com_group_expr_ressortgroupref =
get_com_grouping_ressortgroupref_routine(grp, qry->targetList,com_group_expr_ressortgroupref);
get_com_grouping_ressortgroupref_routine(grp, qry->targetList, com_group_expr_ressortgroupref);

/* Form a list of common refs for grouping clauses. */
com_grouping_ressortgroupref =
Expand Down
3 changes: 2 additions & 1 deletion src/include/nodes/pg_list.h
Original file line number Diff line number Diff line change
Expand Up @@ -240,8 +240,9 @@ extern List *list_union_int(const List *list1, const List *list2);
extern List *list_union_oid(const List *list1, const List *list2);

extern List *list_intersection(const List *list1, const List *list2);
extern List *list_intersection_int(const List *list1, const List *list2);

/* currently, there's no need for list_intersection_int etc */
/* currently, there's no need for list_intersection_ptr etc */

extern List *list_difference(const List *list1, const List *list2);
extern List *list_difference_ptr(const List *list1, const List *list2);
Expand Down