From 7645376774c8532159f5f0f905e5e734d4ccbb18 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 20 Apr 2021 11:37:36 -0400 Subject: [PATCH] Rename find_em_expr_usable_for_sorting_rel. I didn't particularly like this function name, as it fails to express what's going on. Also, returning the sort expression alone isn't too helpful --- typically, a caller would also need some other fields of the EquivalenceMember. But the sole caller really only needs a bool result, so let's make it "bool relation_can_be_sorted_early()". Discussion: https://postgr.es/m/91f3ec99-85a4-fa55-ea74-33f85a5c651f@swarm64.com --- src/backend/optimizer/path/allpaths.c | 21 +++++++++---------- src/backend/optimizer/path/equivclass.c | 27 ++++++++++++++----------- src/include/optimizer/paths.h | 7 +++---- 3 files changed, 28 insertions(+), 27 deletions(-) diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c index edba5e49a8..30728be85a 100644 --- a/src/backend/optimizer/path/allpaths.c +++ b/src/backend/optimizer/path/allpaths.c @@ -2697,20 +2697,19 @@ get_useful_pathkeys_for_relation(PlannerInfo *root, RelOptInfo *rel, EquivalenceClass *pathkey_ec = pathkey->pk_eclass; /* - * We can only build a sort for pathkeys which contain an EC - * member in the current relation's target, so ignore any suffix - * of the list as soon as we find a pathkey without an EC member - * in the relation. + * We can only build a sort for pathkeys that contain a + * safe-to-compute-early EC member computable from the current + * relation's reltarget, so ignore the remainder of the list as + * soon as we find a pathkey without such a member. * - * By still returning the prefix of the pathkeys list that does - * meet criteria of EC membership in the current relation, we - * enable not just an incremental sort on the entirety of - * query_pathkeys but also incremental sort below a JOIN. + * It's still worthwhile to return any prefix of the pathkeys list + * that meets this requirement, as we may be able to do an + * incremental sort. * - * If requested, ensure the expression is parallel safe too. + * If requested, ensure the sort expression is parallel-safe too. */ - if (!find_em_expr_usable_for_sorting_rel(root, pathkey_ec, rel, - require_parallel_safe)) + if (!relation_can_be_sorted_early(root, rel, pathkey_ec, + require_parallel_safe)) break; npathkeys++; diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c index 6e87fba2aa..6f1abbe47d 100644 --- a/src/backend/optimizer/path/equivclass.c +++ b/src/backend/optimizer/path/equivclass.c @@ -961,18 +961,21 @@ find_em_expr_for_rel(EquivalenceClass *ec, RelOptInfo *rel) } /* - * Find an equivalence class member expression that can be used to build - * a sort node using the provided relation; return NULL if no candidate. + * relation_can_be_sorted_early + * Can this relation be sorted on this EC before the final output step? * * To succeed, we must find an EC member that prepare_sort_from_pathkeys knows * how to sort on, given the rel's reltarget as input. There are also a few * additional constraints based on the fact that the desired sort will be done - * within the scan/join part of the plan. Also, non-parallel-safe expressions - * are ignored if 'require_parallel_safe'. + * "early", within the scan/join part of the plan. Also, non-parallel-safe + * expressions are ignored if 'require_parallel_safe'. + * + * At some point we might want to return the identified EquivalenceMember, + * but for now, callers only want to know if there is one. */ -Expr * -find_em_expr_usable_for_sorting_rel(PlannerInfo *root, EquivalenceClass *ec, - RelOptInfo *rel, bool require_parallel_safe) +bool +relation_can_be_sorted_early(PlannerInfo *root, RelOptInfo *rel, + EquivalenceClass *ec, bool require_parallel_safe) { PathTarget *target = rel->reltarget; EquivalenceMember *em; @@ -982,7 +985,7 @@ find_em_expr_usable_for_sorting_rel(PlannerInfo *root, EquivalenceClass *ec, * Reject volatile ECs immediately; such sorts must always be postponed. */ if (ec->ec_has_volatile) - return NULL; + return false; /* * Try to find an EM directly matching some reltarget member. @@ -1012,7 +1015,7 @@ find_em_expr_usable_for_sorting_rel(PlannerInfo *root, EquivalenceClass *ec, !is_parallel_safe(root, (Node *) em->em_expr)) continue; - return em->em_expr; + return true; } /* @@ -1021,7 +1024,7 @@ find_em_expr_usable_for_sorting_rel(PlannerInfo *root, EquivalenceClass *ec, em = find_computable_ec_member(root, ec, target->exprs, rel->relids, require_parallel_safe); if (!em) - return NULL; + return false; /* * Reject expressions involving set-returning functions, as those can't be @@ -1030,9 +1033,9 @@ find_em_expr_usable_for_sorting_rel(PlannerInfo *root, EquivalenceClass *ec, * belong to multi-member ECs.) */ if (IS_SRF_CALL((Node *) em->em_expr)) - return NULL; + return false; - return em->em_expr; + return true; } /* diff --git a/src/include/optimizer/paths.h b/src/include/optimizer/paths.h index 888e85ff5b..f1d111063c 100644 --- a/src/include/optimizer/paths.h +++ b/src/include/optimizer/paths.h @@ -144,10 +144,9 @@ extern EquivalenceMember *find_computable_ec_member(PlannerInfo *root, Relids relids, bool require_parallel_safe); extern Expr *find_em_expr_for_rel(EquivalenceClass *ec, RelOptInfo *rel); -extern Expr *find_em_expr_usable_for_sorting_rel(PlannerInfo *root, - EquivalenceClass *ec, - RelOptInfo *rel, - bool require_parallel_safe); +extern bool relation_can_be_sorted_early(PlannerInfo *root, RelOptInfo *rel, + EquivalenceClass *ec, + bool require_parallel_safe); extern void generate_base_implied_equalities(PlannerInfo *root); extern List *generate_join_implied_equalities(PlannerInfo *root, Relids join_relids,