v45-0005-rework-of-get_useful_pathkeys_for_relation.patch

text/x-patch

Filename: v45-0005-rework-of-get_useful_pathkeys_for_relation.patch
Type: text/x-patch
Part: 1
Message: Re: [PATCH] Incremental sort (was: PoC: Partial sort)

Patch

Format: format-patch
Series: patch v45-0005
Subject: rework of get_useful_pathkeys_for_relation
File+
src/backend/optimizer/path/allpaths.c 18 14
From cf47f29fa4c254bf14d7107e75dfd432350dbfcf Mon Sep 17 00:00:00 2001
From: James Coleman <jtc331@gmail.com>
Date: Sat, 28 Mar 2020 20:03:27 -0400
Subject: [PATCH v45 5/7] rework of get_useful_pathkeys_for_relation

---
 src/backend/optimizer/path/allpaths.c | 32 +++++++++++++++------------
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index 32bf734820..480803fb7a 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -2742,13 +2742,13 @@ generate_gather_paths(PlannerInfo *root, RelOptInfo *rel, bool override_rows)
  * XXX At the moment this can only ever return a list with a single element,
  * because it looks at query_pathkeys only. So we might return the pathkeys
  * directly, but it seems plausible we'll want to consider other orderings
- * in the future.
+ * in the future. For example, we might want to consider pathkeys useful for
+ * merge joins.
  */
 static List *
 get_useful_pathkeys_for_relation(PlannerInfo *root, RelOptInfo *rel)
 {
 	List	   *useful_pathkeys_list = NIL;
-	ListCell   *lc;
 
 	/*
 	 * Considering query_pathkeys is always worth it, because it might allow us
@@ -2756,29 +2756,33 @@ get_useful_pathkeys_for_relation(PlannerInfo *root, RelOptInfo *rel)
 	 */
 	if (root->query_pathkeys)
 	{
-		bool		query_pathkeys_ok = true;
+		ListCell   *lc;
+		List	   *pathkeys = NIL;
 
 		foreach(lc, root->query_pathkeys)
 		{
 			PathKey    *pathkey = (PathKey *) lfirst(lc);
 			EquivalenceClass *pathkey_ec = pathkey->pk_eclass;
-			Expr	   *em_expr;
 
 			/*
-			 * We can't use incremental sort for pathkeys containing volatile
-			 * expressions. We could walk the exppression itself, but checking
-			 * ec_has_volatile here saves some cycles.
+			 * We can only build an Incremental Sort for pathkeys which contain
+			 * an EC member in the current relation, so ignore any suffix of the
+			 * list as soon as we find a pathkey without an EC member the
+			 * relation.
+			 *
+			 * 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.
 			 */
-			if (pathkey_ec->ec_has_volatile ||
-				!(em_expr = find_em_expr_for_rel(pathkey_ec, rel)))
-			{
-				query_pathkeys_ok = false;
+			if (!find_em_expr_for_rel(pathkey_ec, rel))
 				break;
-			}
+
+			pathkeys = lappend(pathkeys, pathkey);
 		}
 
-		if (query_pathkeys_ok)
-			useful_pathkeys_list = list_make1(list_copy(root->query_pathkeys));
+		if (pathkeys)
+			useful_pathkeys_list = lappend(useful_pathkeys_list, pathkeys);
 	}
 
 	return useful_pathkeys_list;
-- 
2.17.1