database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-09-09 13:09:39 +03:00
Code Activity

Fix usage of aggregate pathkeys in group_keys_reorder_by_pathkeys()

Browse Source 
group_keys_reorder_by_pathkeys() function searched for matching pathkeys
within root->group_pathkeys.  That could lead to picking an aggregate pathkey
and using its pathkey->pk_eclass->ec_sortref as an argument of
get_sortgroupref_clause_noerr().  Given that ec_sortref of an aggregate pathkey
references aggregate targetlist not query targetlist, this leads to incorrect
query optimization.

Fix this by looking for matching pathkeys only within the first
num_groupby_pathkeys pathkeys.

Reported-by: David G. Johnston
Discussion: https://postgr.es/m/CAKFQuwY3Ek%3DcLThgd8FdaSc5JRDVt0FaV00gMcWra%2BTAR4gGUw%40mail.gmail.com
Author: Andrei Lepikhov, Alexander Korotkov
This commit is contained in:
Alexander Korotkov
2024-02-09 12:56:26 +02:00
parent 6743c5ae64
commit c01f6ef46c
3 changed files with 64 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
src/backend/optimizer/path/pathkeys.c
View File

@@ -355,10 +355,14 @@ pathkeys_contained_in(List *keys1, List *keys2)
/*
* group_keys_reorder_by_pathkeys
* Reorder GROUP BY keys to match the input pathkeys.
* Reorder GROUP BY pathkeys and clauses to match the input pathkeys.
*
* Function returns new lists (pathkeys and clauses), original GROUP BY lists
* stay untouched.
* 'pathkeys' is an input list of pathkeys
* '*group_pathkeys' and '*group_clauses' are pathkeys and clauses lists to
* reorder. The pointers are redirected to new lists, original lists
* stay untouched.
* 'num_groupby_pathkeys' is the number of first '*group_pathkeys' items to
* search matching pathkeys.
*
* Returns the number of GROUP BY keys with a matching pathkey.
*/
@@ -369,12 +373,24 @@ group_keys_reorder_by_pathkeys(List *pathkeys, List **group_pathkeys,
{
List *new_group_pathkeys = NIL,
*new_group_clauses = NIL;
List *grouping_pathkeys;
ListCell *lc;
int n;
if (pathkeys == NIL || *group_pathkeys == NIL)
return 0;
/*
* We're going to search within just the first num_groupby_pathkeys of
* *group_pathkeys. The thing is that root->group_pathkeys is passed as
* *group_pathkeys containing grouping pathkeys altogether with aggregate
* pathkeys. If we process aggregate pathkeys we could get an invalid
* result of get_sortgroupref_clause_noerr(), because their
* pathkey->pk_eclass->ec_sortref doesn't referece query targetlist. So,
* we allocate a separate list of pathkeys for lookups.
*/
grouping_pathkeys = list_copy_head(*group_pathkeys, num_groupby_pathkeys);
/*
* Walk the pathkeys (determining ordering of the input path) and see if
* there's a matching GROUP BY key. If we find one, we append it to the
@@ -395,7 +411,7 @@ group_keys_reorder_by_pathkeys(List *pathkeys, List **group_pathkeys,
* there is no sortclause reference for some reason.
*/
if (foreach_current_index(lc) >= num_groupby_pathkeys ||
!list_member_ptr(*group_pathkeys, pathkey) ||
!list_member_ptr(grouping_pathkeys, pathkey) ||
pathkey->pk_eclass->ec_sortref == 0)
break;
@@ -429,6 +445,7 @@ group_keys_reorder_by_pathkeys(List *pathkeys, List **group_pathkeys,
*group_clauses = list_concat_unique_ptr(new_group_clauses,
*group_clauses);
list_free(grouping_pathkeys);
return n;
}