diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c index 21ce1ae2e1..51d806326e 100644 --- a/src/backend/optimizer/path/equivclass.c +++ b/src/backend/optimizer/path/equivclass.c @@ -652,18 +652,7 @@ get_eclass_for_sort_expr(PlannerInfo *root, if (opcintype == cur_em->em_datatype && equal(expr, cur_em->em_expr)) - { - /* - * Match! - * - * Copy the sortref if it wasn't set yet. That may happen if - * the ec was constructed from a WHERE clause, i.e. it doesn't - * have a target reference at all. - */ - if (cur_ec->ec_sortref == 0 && sortref > 0) - cur_ec->ec_sortref = sortref; - return cur_ec; - } + return cur_ec; /* Match! */ } } diff --git a/src/backend/optimizer/path/pathkeys.c b/src/backend/optimizer/path/pathkeys.c index 8b258cbef9..df966b18f3 100644 --- a/src/backend/optimizer/path/pathkeys.c +++ b/src/backend/optimizer/path/pathkeys.c @@ -1355,7 +1355,8 @@ make_pathkeys_for_sortclauses(PlannerInfo *root, &sortclauses, tlist, false, - &sortable); + &sortable, + false); /* It's caller error if not all clauses were sortable */ Assert(sortable); return result; @@ -1379,13 +1380,17 @@ make_pathkeys_for_sortclauses(PlannerInfo *root, * to remove any clauses that can be proven redundant via the eclass logic. * Even though we'll have to hash in that case, we might as well not hash * redundant columns.) + * + * If set_ec_sortref is true then sets the value of the pathkey's + * EquivalenceClass unless it's already initialized. */ List * make_pathkeys_for_sortclauses_extended(PlannerInfo *root, List **sortclauses, List *tlist, bool remove_redundant, - bool *sortable) + bool *sortable, + bool set_ec_sortref) { List *pathkeys = NIL; ListCell *l; @@ -1409,6 +1414,15 @@ make_pathkeys_for_sortclauses_extended(PlannerInfo *root, sortcl->nulls_first, sortcl->tleSortGroupRef, true); + if (pathkey->pk_eclass->ec_sortref == 0 && set_ec_sortref) + { + /* + * Copy the sortref if it hasn't been set yet. That may happen if + * the EquivalenceClass was constructed from a WHERE clause, i.e. + * it doesn't have a target reference at all. + */ + pathkey->pk_eclass->ec_sortref = sortcl->tleSortGroupRef; + } /* Canonical form eliminates redundant ordering keys */ if (!pathkey_is_redundant(pathkey, pathkeys)) diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 032818423f..ea107c70cb 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -3395,12 +3395,17 @@ standard_qp_callback(PlannerInfo *root, void *extra) */ bool sortable; + /* + * Convert group clauses into pathkeys. Set the ec_sortref field of + * EquivalenceClass'es if it's not set yet. + */ root->group_pathkeys = make_pathkeys_for_sortclauses_extended(root, &root->processed_groupClause, tlist, true, - &sortable); + &sortable, + true); if (!sortable) { /* Can't sort; no point in considering aggregate ordering either */ @@ -3450,7 +3455,8 @@ standard_qp_callback(PlannerInfo *root, void *extra) &root->processed_distinctClause, tlist, true, - &sortable); + &sortable, + false); if (!sortable) root->distinct_pathkeys = NIL; } @@ -3476,7 +3482,8 @@ standard_qp_callback(PlannerInfo *root, void *extra) &groupClauses, tlist, false, - &sortable); + &sortable, + false); if (!sortable) root->setop_pathkeys = NIL; } @@ -6061,7 +6068,8 @@ make_pathkeys_for_window(PlannerInfo *root, WindowClause *wc, &wc->partitionClause, tlist, true, - &sortable); + &sortable, + false); Assert(sortable); } diff --git a/src/include/optimizer/paths.h b/src/include/optimizer/paths.h index 914d9bdef5..5e88c0224a 100644 --- a/src/include/optimizer/paths.h +++ b/src/include/optimizer/paths.h @@ -239,7 +239,8 @@ extern List *make_pathkeys_for_sortclauses_extended(PlannerInfo *root, List **sortclauses, List *tlist, bool remove_redundant, - bool *sortable); + bool *sortable, + bool set_ec_sortref); extern void initialize_mergeclause_eclasses(PlannerInfo *root, RestrictInfo *restrictinfo); extern void update_mergeclause_eclasses(PlannerInfo *root, diff --git a/src/test/regress/expected/aggregates.out b/src/test/regress/expected/aggregates.out index 2442342e9d..1c1ca7573a 100644 --- a/src/test/regress/expected/aggregates.out +++ b/src/test/regress/expected/aggregates.out @@ -2917,6 +2917,53 @@ GROUP BY c1.w, c1.z; 5.0000000000000000 (2 rows) +-- Pathkeys, built in a subtree, can be used to optimize GROUP-BY clause +-- ordering. Also, here we check that it doesn't depend on the initial clause +-- order in the GROUP-BY list. +EXPLAIN (COSTS OFF) +SELECT c1.y,c1.x FROM group_agg_pk c1 + JOIN group_agg_pk c2 + ON c1.x = c2.x +GROUP BY c1.y,c1.x,c2.x; + QUERY PLAN +----------------------------------------------------- + Group + Group Key: c1.x, c1.y + -> Incremental Sort + Sort Key: c1.x, c1.y + Presorted Key: c1.x + -> Merge Join + Merge Cond: (c1.x = c2.x) + -> Sort + Sort Key: c1.x + -> Seq Scan on group_agg_pk c1 + -> Sort + Sort Key: c2.x + -> Seq Scan on group_agg_pk c2 +(13 rows) + +EXPLAIN (COSTS OFF) +SELECT c1.y,c1.x FROM group_agg_pk c1 + JOIN group_agg_pk c2 + ON c1.x = c2.x +GROUP BY c1.y,c2.x,c1.x; + QUERY PLAN +----------------------------------------------------- + Group + Group Key: c2.x, c1.y + -> Incremental Sort + Sort Key: c2.x, c1.y + Presorted Key: c2.x + -> Merge Join + Merge Cond: (c1.x = c2.x) + -> Sort + Sort Key: c1.x + -> Seq Scan on group_agg_pk c1 + -> Sort + Sort Key: c2.x + -> Seq Scan on group_agg_pk c2 +(13 rows) + RESET enable_nestloop; RESET enable_hashjoin; DROP TABLE group_agg_pk; diff --git a/src/test/regress/sql/aggregates.sql b/src/test/regress/sql/aggregates.sql index 61a3424c84..1a18ca3d8f 100644 --- a/src/test/regress/sql/aggregates.sql +++ b/src/test/regress/sql/aggregates.sql @@ -1263,6 +1263,20 @@ SELECT avg(c1.f ORDER BY c1.x, c1.y) FROM group_agg_pk c1 JOIN group_agg_pk c2 ON c1.x = c2.x GROUP BY c1.w, c1.z; +-- Pathkeys, built in a subtree, can be used to optimize GROUP-BY clause +-- ordering. Also, here we check that it doesn't depend on the initial clause +-- order in the GROUP-BY list. +EXPLAIN (COSTS OFF) +SELECT c1.y,c1.x FROM group_agg_pk c1 + JOIN group_agg_pk c2 + ON c1.x = c2.x +GROUP BY c1.y,c1.x,c2.x; +EXPLAIN (COSTS OFF) +SELECT c1.y,c1.x FROM group_agg_pk c1 + JOIN group_agg_pk c2 + ON c1.x = c2.x +GROUP BY c1.y,c2.x,c1.x; + RESET enable_nestloop; RESET enable_hashjoin; DROP TABLE group_agg_pk;