Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitc01f6ef

Browse files
committed
Fix usage of aggregate pathkeys in group_keys_reorder_by_pathkeys()
group_keys_reorder_by_pathkeys() function searched for matching pathkeyswithin root->group_pathkeys. That could lead to picking an aggregate pathkeyand using its pathkey->pk_eclass->ec_sortref as an argument ofget_sortgroupref_clause_noerr(). Given that ec_sortref of an aggregate pathkeyreferences aggregate targetlist not query targetlist, this leads to incorrectquery optimization.Fix this by looking for matching pathkeys only within the firstnum_groupby_pathkeys pathkeys.Reported-by: David G. JohnstonDiscussion:https://postgr.es/m/CAKFQuwY3Ek%3DcLThgd8FdaSc5JRDVt0FaV00gMcWra%2BTAR4gGUw%40mail.gmail.comAuthor: Andrei Lepikhov, Alexander Korotkov
1 parent6743c5a commitc01f6ef

File tree

3 files changed

+64
-4
lines changed

3 files changed

+64
-4
lines changed

‎src/backend/optimizer/path/pathkeys.c

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -355,10 +355,14 @@ pathkeys_contained_in(List *keys1, List *keys2)
355355

356356
/*
357357
* group_keys_reorder_by_pathkeys
358-
*Reorder GROUP BYkeys to match the input pathkeys.
358+
*Reorder GROUP BYpathkeys and clauses to match the input pathkeys.
359359
*
360-
* Function returns new lists (pathkeys and clauses), original GROUP BY lists
361-
* stay untouched.
360+
* 'pathkeys' is an input list of pathkeys
361+
* '*group_pathkeys' and '*group_clauses' are pathkeys and clauses lists to
362+
*reorder. The pointers are redirected to new lists, original lists
363+
*stay untouched.
364+
* 'num_groupby_pathkeys' is the number of first '*group_pathkeys' items to
365+
*search matching pathkeys.
362366
*
363367
* Returns the number of GROUP BY keys with a matching pathkey.
364368
*/
@@ -369,12 +373,24 @@ group_keys_reorder_by_pathkeys(List *pathkeys, List **group_pathkeys,
369373
{
370374
List*new_group_pathkeys=NIL,
371375
*new_group_clauses=NIL;
376+
List*grouping_pathkeys;
372377
ListCell*lc;
373378
intn;
374379

375380
if (pathkeys==NIL||*group_pathkeys==NIL)
376381
return0;
377382

383+
/*
384+
* We're going to search within just the first num_groupby_pathkeys of
385+
* *group_pathkeys. The thing is that root->group_pathkeys is passed as
386+
* *group_pathkeys containing grouping pathkeys altogether with aggregate
387+
* pathkeys. If we process aggregate pathkeys we could get an invalid
388+
* result of get_sortgroupref_clause_noerr(), because their
389+
* pathkey->pk_eclass->ec_sortref doesn't referece query targetlist. So,
390+
* we allocate a separate list of pathkeys for lookups.
391+
*/
392+
grouping_pathkeys=list_copy_head(*group_pathkeys,num_groupby_pathkeys);
393+
378394
/*
379395
* Walk the pathkeys (determining ordering of the input path) and see if
380396
* 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,
395411
* there is no sortclause reference for some reason.
396412
*/
397413
if (foreach_current_index(lc) >=num_groupby_pathkeys||
398-
!list_member_ptr(*group_pathkeys,pathkey)||
414+
!list_member_ptr(grouping_pathkeys,pathkey)||
399415
pathkey->pk_eclass->ec_sortref==0)
400416
break;
401417

@@ -429,6 +445,7 @@ group_keys_reorder_by_pathkeys(List *pathkeys, List **group_pathkeys,
429445
*group_clauses=list_concat_unique_ptr(new_group_clauses,
430446
*group_clauses);
431447

448+
list_free(grouping_pathkeys);
432449
returnn;
433450
}
434451

‎src/test/regress/expected/aggregates.out

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2874,6 +2874,30 @@ SELECT y,x,array_agg(distinct w) FROM btg WHERE y < 0 GROUP BY x,y;
28742874

28752875
RESET enable_incremental_sort;
28762876
DROP TABLE btg;
2877+
-- Check we don't pick aggregate path key instead of grouping path key
2878+
CREATE TABLE group_agg_pk AS SELECT
2879+
i % 10 AS x,
2880+
i % 2 AS y,
2881+
i % 2 AS z,
2882+
2 AS w,
2883+
i % 10 AS f
2884+
FROM generate_series(1,100) AS i;
2885+
ANALYZE group_agg_pk;
2886+
SET enable_nestloop = off;
2887+
SET enable_hashjoin = off;
2888+
SELECT
2889+
c1.z, c1.w, string_agg(''::text, repeat(''::text, c1.f) ORDER BY c1.x,c1.y)
2890+
FROM group_agg_pk c1 JOIN group_agg_pk c2 ON (c1.x = c2.f)
2891+
GROUP BY c1.w, c1.z;
2892+
z | w | string_agg
2893+
---+---+------------
2894+
0 | 2 |
2895+
1 | 2 |
2896+
(2 rows)
2897+
2898+
RESET enable_nestloop;
2899+
RESET enable_hashjoin;
2900+
DROP TABLE group_agg_pk;
28772901
-- The case, when scanning sort order correspond to aggregate sort order but
28782902
-- can not be found in the group-by list
28792903
CREATE TABLE agg_sort_order (c1 int PRIMARY KEY, c2 int);

‎src/test/regress/sql/aggregates.sql

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1231,6 +1231,25 @@ RESET enable_incremental_sort;
12311231

12321232
DROPTABLE btg;
12331233

1234+
-- Check we don't pick aggregate path key instead of grouping path key
1235+
CREATETABLEgroup_agg_pkASSELECT
1236+
i %10AS x,
1237+
i %2AS y,
1238+
i %2AS z,
1239+
2AS w,
1240+
i %10AS f
1241+
FROM generate_series(1,100)AS i;
1242+
ANALYZE group_agg_pk;
1243+
SET enable_nestloop= off;
1244+
SET enable_hashjoin= off;
1245+
SELECT
1246+
c1.z,c1.w, string_agg(''::text, repeat(''::text,c1.f)ORDER BYc1.x,c1.y)
1247+
FROM group_agg_pk c1JOIN group_agg_pk c2ON (c1.x=c2.f)
1248+
GROUP BYc1.w,c1.z;
1249+
RESET enable_nestloop;
1250+
RESET enable_hashjoin;
1251+
DROPTABLE group_agg_pk;
1252+
12341253
-- The case, when scanning sort order correspond to aggregate sort order but
12351254
-- can not be found in the group-by list
12361255
CREATETABLEagg_sort_order (c1intPRIMARY KEY, c2int);

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp