Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 199012a

Browse files
committed
Fix asymmetry in setting EquivalenceClass.ec_sortref
0452b46 made get_eclass_for_sort_expr() always set EquivalenceClass.ec_sortref if it's not done yet. This leads to an asymmetric situation when whoever first looks for the EquivalenceClass sets the ec_sortref. It is also counterintuitive that get_eclass_for_sort_expr() performs modification of data structures. This commit makes make_pathkeys_for_sortclauses_extended() responsible for setting EquivalenceClass.ec_sortref. Now we set the EquivalenceClass.ec_sortref's needed to explore alternative GROUP BY ordering specifically during building pathkeys by the list of grouping clauses. Discussion: https://postgr.es/m/17037754-f187-4138-8285-0e2bfebd0dea%40postgrespro.ru Reported-by: Tom Lane Author: Andrei Lepikhov Reviewed-by: Alexander Korotkov, Pavel Borisov
1 parent f654f00 commit 199012a

File tree

6 files changed

+92
-19
lines changed

6 files changed

+92
-19
lines changed

src/backend/optimizer/path/equivclass.c

+1-12
Original file line numberDiff line numberDiff line change
@@ -652,18 +652,7 @@ get_eclass_for_sort_expr(PlannerInfo *root,
652652

653653
if (opcintype == cur_em->em_datatype &&
654654
equal(expr, cur_em->em_expr))
655-
{
656-
/*
657-
* Match!
658-
*
659-
* Copy the sortref if it wasn't set yet. That may happen if
660-
* the ec was constructed from a WHERE clause, i.e. it doesn't
661-
* have a target reference at all.
662-
*/
663-
if (cur_ec->ec_sortref == 0 && sortref > 0)
664-
cur_ec->ec_sortref = sortref;
665-
return cur_ec;
666-
}
655+
return cur_ec; /* Match! */
667656
}
668657
}
669658

src/backend/optimizer/path/pathkeys.c

+16-2
Original file line numberDiff line numberDiff line change
@@ -1355,7 +1355,8 @@ make_pathkeys_for_sortclauses(PlannerInfo *root,
13551355
&sortclauses,
13561356
tlist,
13571357
false,
1358-
&sortable);
1358+
&sortable,
1359+
false);
13591360
/* It's caller error if not all clauses were sortable */
13601361
Assert(sortable);
13611362
return result;
@@ -1379,13 +1380,17 @@ make_pathkeys_for_sortclauses(PlannerInfo *root,
13791380
* to remove any clauses that can be proven redundant via the eclass logic.
13801381
* Even though we'll have to hash in that case, we might as well not hash
13811382
* redundant columns.)
1383+
*
1384+
* If set_ec_sortref is true then sets the value of the pathkey's
1385+
* EquivalenceClass unless it's already initialized.
13821386
*/
13831387
List *
13841388
make_pathkeys_for_sortclauses_extended(PlannerInfo *root,
13851389
List **sortclauses,
13861390
List *tlist,
13871391
bool remove_redundant,
1388-
bool *sortable)
1392+
bool *sortable,
1393+
bool set_ec_sortref)
13891394
{
13901395
List *pathkeys = NIL;
13911396
ListCell *l;
@@ -1409,6 +1414,15 @@ make_pathkeys_for_sortclauses_extended(PlannerInfo *root,
14091414
sortcl->nulls_first,
14101415
sortcl->tleSortGroupRef,
14111416
true);
1417+
if (pathkey->pk_eclass->ec_sortref == 0 && set_ec_sortref)
1418+
{
1419+
/*
1420+
* Copy the sortref if it hasn't been set yet. That may happen if
1421+
* the EquivalenceClass was constructed from a WHERE clause, i.e.
1422+
* it doesn't have a target reference at all.
1423+
*/
1424+
pathkey->pk_eclass->ec_sortref = sortcl->tleSortGroupRef;
1425+
}
14121426

14131427
/* Canonical form eliminates redundant ordering keys */
14141428
if (!pathkey_is_redundant(pathkey, pathkeys))

src/backend/optimizer/plan/planner.c

+12-4
Original file line numberDiff line numberDiff line change
@@ -3395,12 +3395,17 @@ standard_qp_callback(PlannerInfo *root, void *extra)
33953395
*/
33963396
bool sortable;
33973397

3398+
/*
3399+
* Convert group clauses into pathkeys. Set the ec_sortref field of
3400+
* EquivalenceClass'es if it's not set yet.
3401+
*/
33983402
root->group_pathkeys =
33993403
make_pathkeys_for_sortclauses_extended(root,
34003404
&root->processed_groupClause,
34013405
tlist,
34023406
true,
3403-
&sortable);
3407+
&sortable,
3408+
true);
34043409
if (!sortable)
34053410
{
34063411
/* Can't sort; no point in considering aggregate ordering either */
@@ -3450,7 +3455,8 @@ standard_qp_callback(PlannerInfo *root, void *extra)
34503455
&root->processed_distinctClause,
34513456
tlist,
34523457
true,
3453-
&sortable);
3458+
&sortable,
3459+
false);
34543460
if (!sortable)
34553461
root->distinct_pathkeys = NIL;
34563462
}
@@ -3476,7 +3482,8 @@ standard_qp_callback(PlannerInfo *root, void *extra)
34763482
&groupClauses,
34773483
tlist,
34783484
false,
3479-
&sortable);
3485+
&sortable,
3486+
false);
34803487
if (!sortable)
34813488
root->setop_pathkeys = NIL;
34823489
}
@@ -6061,7 +6068,8 @@ make_pathkeys_for_window(PlannerInfo *root, WindowClause *wc,
60616068
&wc->partitionClause,
60626069
tlist,
60636070
true,
6064-
&sortable);
6071+
&sortable,
6072+
false);
60656073

60666074
Assert(sortable);
60676075
}

src/include/optimizer/paths.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,8 @@ extern List *make_pathkeys_for_sortclauses_extended(PlannerInfo *root,
239239
List **sortclauses,
240240
List *tlist,
241241
bool remove_redundant,
242-
bool *sortable);
242+
bool *sortable,
243+
bool set_ec_sortref);
243244
extern void initialize_mergeclause_eclasses(PlannerInfo *root,
244245
RestrictInfo *restrictinfo);
245246
extern void update_mergeclause_eclasses(PlannerInfo *root,

src/test/regress/expected/aggregates.out

+47
Original file line numberDiff line numberDiff line change
@@ -2917,6 +2917,53 @@ GROUP BY c1.w, c1.z;
29172917
5.0000000000000000
29182918
(2 rows)
29192919

2920+
-- Pathkeys, built in a subtree, can be used to optimize GROUP-BY clause
2921+
-- ordering. Also, here we check that it doesn't depend on the initial clause
2922+
-- order in the GROUP-BY list.
2923+
EXPLAIN (COSTS OFF)
2924+
SELECT c1.y,c1.x FROM group_agg_pk c1
2925+
JOIN group_agg_pk c2
2926+
ON c1.x = c2.x
2927+
GROUP BY c1.y,c1.x,c2.x;
2928+
QUERY PLAN
2929+
-----------------------------------------------------
2930+
Group
2931+
Group Key: c1.x, c1.y
2932+
-> Incremental Sort
2933+
Sort Key: c1.x, c1.y
2934+
Presorted Key: c1.x
2935+
-> Merge Join
2936+
Merge Cond: (c1.x = c2.x)
2937+
-> Sort
2938+
Sort Key: c1.x
2939+
-> Seq Scan on group_agg_pk c1
2940+
-> Sort
2941+
Sort Key: c2.x
2942+
-> Seq Scan on group_agg_pk c2
2943+
(13 rows)
2944+
2945+
EXPLAIN (COSTS OFF)
2946+
SELECT c1.y,c1.x FROM group_agg_pk c1
2947+
JOIN group_agg_pk c2
2948+
ON c1.x = c2.x
2949+
GROUP BY c1.y,c2.x,c1.x;
2950+
QUERY PLAN
2951+
-----------------------------------------------------
2952+
Group
2953+
Group Key: c2.x, c1.y
2954+
-> Incremental Sort
2955+
Sort Key: c2.x, c1.y
2956+
Presorted Key: c2.x
2957+
-> Merge Join
2958+
Merge Cond: (c1.x = c2.x)
2959+
-> Sort
2960+
Sort Key: c1.x
2961+
-> Seq Scan on group_agg_pk c1
2962+
-> Sort
2963+
Sort Key: c2.x
2964+
-> Seq Scan on group_agg_pk c2
2965+
(13 rows)
2966+
29202967
RESET enable_nestloop;
29212968
RESET enable_hashjoin;
29222969
DROP TABLE group_agg_pk;

src/test/regress/sql/aggregates.sql

+14
Original file line numberDiff line numberDiff line change
@@ -1263,6 +1263,20 @@ SELECT avg(c1.f ORDER BY c1.x, c1.y)
12631263
FROM group_agg_pk c1 JOIN group_agg_pk c2 ON c1.x = c2.x
12641264
GROUP BY c1.w, c1.z;
12651265

1266+
-- Pathkeys, built in a subtree, can be used to optimize GROUP-BY clause
1267+
-- ordering. Also, here we check that it doesn't depend on the initial clause
1268+
-- order in the GROUP-BY list.
1269+
EXPLAIN (COSTS OFF)
1270+
SELECT c1.y,c1.x FROM group_agg_pk c1
1271+
JOIN group_agg_pk c2
1272+
ON c1.x = c2.x
1273+
GROUP BY c1.y,c1.x,c2.x;
1274+
EXPLAIN (COSTS OFF)
1275+
SELECT c1.y,c1.x FROM group_agg_pk c1
1276+
JOIN group_agg_pk c2
1277+
ON c1.x = c2.x
1278+
GROUP BY c1.y,c2.x,c1.x;
1279+
12661280
RESET enable_nestloop;
12671281
RESET enable_hashjoin;
12681282
DROP TABLE group_agg_pk;

0 commit comments

Comments
 (0)