Rename find_em_expr_usable_for_sorting_rel.

tglsfdc · tglsfdc · commit 7645376774c8 · 2021-04-20T11:37:36.000-04:00
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
diff --git 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
@@ -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
@@ -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,
