0002-Combine-reparameterize_path_by_child-and-path_is_rep.patch

From 454ad31f88b51d6f821cf22a55008e7b5fdb4ce3 Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
Date: Tue, 22 Aug 2023 21:27:39 +0530
Subject: [PATCH 2/2] Combine reparameterize_path_by_child and
 path_is_reparameterizable_by_child

reparameterize_path_by_child works in two modes 1. to assess whether a given path can be reparameterized by child 2. actually reparameterizing it.
---
 src/backend/optimizer/path/joinpath.c   |   6 +-
 src/backend/optimizer/plan/createplan.c |   3 +-
 src/backend/optimizer/util/pathnode.c   | 208 +++++++-----------------
 src/include/foreign/fdwapi.h            |   3 +-
 src/include/nodes/extensible.h          |   3 +-
 src/include/optimizer/pathnode.h        |   5 +-
 6 files changed, 70 insertions(+), 158 deletions(-)

diff --git a/src/backend/optimizer/path/joinpath.c b/src/backend/optimizer/path/joinpath.c
index e2ecf5b14b..6bf16213fe 100644
--- a/src/backend/optimizer/path/joinpath.c
+++ b/src/backend/optimizer/path/joinpath.c
@@ -771,7 +771,8 @@ try_nestloop_path(PlannerInfo *root,
 	 * the translation, and if not avoid creating a nestloop path.
 	 */
 	if (PATH_PARAM_BY_PARENT(inner_path, outer_path->parent) &&
-		!path_is_reparameterizable_by_child(inner_path, outer_path->parent))
+		!path_is_reparameterizable_by_child(root, inner_path,
+											outer_path->parent))
 	{
 		bms_free(required_outer);
 		return;
@@ -863,7 +864,8 @@ try_partial_nestloop_path(PlannerInfo *root,
 	 * the translation, and if not avoid creating a nestloop path.
 	 */
 	if (PATH_PARAM_BY_PARENT(inner_path, outer_path->parent) &&
-		!path_is_reparameterizable_by_child(inner_path, outer_path->parent))
+		!path_is_reparameterizable_by_child(root, inner_path,
+											outer_path->parent))
 		return;
 
 	/*
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 38bd179f4f..29b7af94a8 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -4355,7 +4355,8 @@ create_nestloop_plan(PlannerInfo *root,
 	best_path->jpath.innerjoinpath =
 		reparameterize_path_by_child(root,
 									 best_path->jpath.innerjoinpath,
-									 best_path->jpath.outerjoinpath->parent);
+									 best_path->jpath.outerjoinpath->parent,
+									 false);
 
 	/*
 	 * Failure here probably means that reparameterize_path_by_child() is not
diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index 21d002243c..6eec0bf444 100644
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -55,9 +55,8 @@ static int	append_total_cost_compare(const ListCell *a, const ListCell *b);
 static int	append_startup_cost_compare(const ListCell *a, const ListCell *b);
 static List *reparameterize_pathlist_by_child(PlannerInfo *root,
 											  List *pathlist,
-											  RelOptInfo *child_rel);
-static bool pathlist_is_reparameterizable_by_child(List *pathlist,
-												   RelOptInfo *child_rel);
+											  RelOptInfo *child_rel,
+											  bool check_only);
 
 
 /*****************************************************************************
@@ -4059,26 +4058,45 @@ reparameterize_path(PlannerInfo *root, Path *path,
  * structures.  Therefore, it's only safe to call during create_plan(),
  * when we have made a final choice of which Path to use for each RTE.
  *
+ * If check_only is true, the path is returned as is without any
+ * reparameterization if reparameterization is possible. If check_only is
+ * false, a reparameterized path is returned.
+ *
  * Keep this code in sync with path_is_reparameterizable_by_child()!
  */
 Path *
 reparameterize_path_by_child(PlannerInfo *root, Path *path,
-							 RelOptInfo *child_rel)
+							 RelOptInfo *child_rel, bool check_only)
 {
 
 #define FLAT_COPY_PATH(newnode, node, nodetype)  \
-	( (newnode) = makeNode(nodetype), \
-	  memcpy((newnode), (node), sizeof(nodetype)) )
+do { \
+	if (!(check_only)) \
+	{ \
+		(newnode) = makeNode(nodetype); \
+	  	memcpy((newnode), (node), sizeof(nodetype)); \
+	} \
+	else \
+	{ \
+		(newnode) = (nodetype *) (node); \
+	} \
+} while (0)
 
 #define ADJUST_CHILD_ATTRS(node) \
-	((node) = \
-	 (void *) adjust_appendrel_attrs_multilevel(root, (Node *) (node), \
+do { \
+	if (!(check_only)) \
+	{ \
+		(node) = \
+			 (void *) adjust_appendrel_attrs_multilevel(root, (Node *) (node), \
 												child_rel, \
-												child_rel->top_parent))
+												child_rel->top_parent); \
+	} \
+} while (0)
 
 #define REPARAMETERIZE_CHILD_PATH(path) \
 do { \
-	(path) = reparameterize_path_by_child(root, (path), child_rel); \
+	(path) = \
+		reparameterize_path_by_child(root, (path), child_rel, (check_only)); \
 	if ((path) == NULL) \
 		return NULL; \
 } while(0)
@@ -4088,7 +4106,8 @@ do { \
 	if ((pathlist) != NIL) \
 	{ \
 		(pathlist) = reparameterize_pathlist_by_child(root, (pathlist), \
-													  child_rel); \
+													  child_rel, \
+													  (check_only)); \
 		if ((pathlist) == NIL) \
 			return NULL; \
 	} \
@@ -4123,7 +4142,7 @@ do { \
 	{
 		case T_Path:
 			FLAT_COPY_PATH(new_path, path, Path);
-			if (path->pathtype == T_SampleScan)
+			if (path->pathtype == T_SampleScan && !check_only)
 			{
 				Index		scan_relid = path->parent->relid;
 				RangeTblEntry *rte;
@@ -4194,7 +4213,7 @@ do { \
 					path->parent->fdwroutine->ReparameterizeForeignPathByChild;
 				if (rfpc_func)
 					fpath->fdw_private = rfpc_func(root, fpath->fdw_private,
-												   child_rel);
+												   child_rel, check_only);
 				new_path = (Path *) fpath;
 			}
 			break;
@@ -4212,7 +4231,8 @@ do { \
 					cpath->custom_private =
 						cpath->methods->ReparameterizeCustomPathByChild(root,
 																		cpath->custom_private,
-																		child_rel);
+																		child_rel,
+																		check_only);
 				new_path = (Path *) cpath;
 			}
 			break;
@@ -4311,6 +4331,16 @@ do { \
 			return NULL;
 	}
 
+	/*
+	 * If we are here to only assess whether reparameterization is possible, we
+	 * are done. In this mode we shouldn't be creating a new path node.
+	 */
+	if (check_only)
+	{
+		Assert(new_path == path);
+		return new_path;
+	}
+
 	/*
 	 * Adjust the parameterization information, which refers to the topmost
 	 * parent. The topmost parent can be multiple levels away from the given
@@ -4378,125 +4408,20 @@ do { \
  * disregarded since they won't require translation.
  */
 bool
-path_is_reparameterizable_by_child(Path *path, RelOptInfo *child_rel)
+path_is_reparameterizable_by_child(PlannerInfo *root, Path *path,
+								   RelOptInfo *child_rel)
 {
+	Path	   *result;
 
-#define CHILD_PATH_IS_REPARAMETERIZABLE(path) \
-do { \
-	if (!path_is_reparameterizable_by_child(path, child_rel)) \
-		return false; \
-} while(0)
-
-#define CHILD_PATH_LIST_IS_REPARAMETERIZABLE(pathlist) \
-do { \
-	if (!pathlist_is_reparameterizable_by_child(pathlist, child_rel)) \
-		return false; \
-} while(0)
+	result = reparameterize_path_by_child(root, path, child_rel, true);
+	if (!result)
+		return false;
 
 	/*
-	 * If the path is not parameterized by the parent of the given relation,
-	 * it doesn't need reparameterization.
+	 * Original path should be returned as is when we are assessing possibility
+	 * of reparameterization.
 	 */
-	if (!path->param_info ||
-		!bms_overlap(PATH_REQ_OUTER(path), child_rel->top_parent_relids))
-		return true;
-
-	switch (nodeTag(path))
-	{
-		case T_Path:
-		case T_IndexPath:
-			break;
-
-		case T_BitmapHeapPath:
-			{
-				BitmapHeapPath *bhpath = (BitmapHeapPath *) path;
-
-				CHILD_PATH_IS_REPARAMETERIZABLE(bhpath->bitmapqual);
-			}
-			break;
-
-		case T_BitmapAndPath:
-			{
-				BitmapAndPath *bapath = (BitmapAndPath *) path;
-
-				CHILD_PATH_LIST_IS_REPARAMETERIZABLE(bapath->bitmapquals);
-			}
-			break;
-
-		case T_BitmapOrPath:
-			{
-				BitmapOrPath *bopath = (BitmapOrPath *) path;
-
-				CHILD_PATH_LIST_IS_REPARAMETERIZABLE(bopath->bitmapquals);
-			}
-			break;
-
-		case T_ForeignPath:
-			{
-				ForeignPath *fpath = (ForeignPath *) path;
-
-				if (fpath->fdw_outerpath)
-					CHILD_PATH_IS_REPARAMETERIZABLE(fpath->fdw_outerpath);
-			}
-			break;
-
-		case T_CustomPath:
-			{
-				CustomPath *cpath = (CustomPath *) path;
-
-				CHILD_PATH_LIST_IS_REPARAMETERIZABLE(cpath->custom_paths);
-			}
-			break;
-
-		case T_NestPath:
-		case T_MergePath:
-		case T_HashPath:
-			{
-				JoinPath   *jpath = (JoinPath *) path;
-
-				CHILD_PATH_IS_REPARAMETERIZABLE(jpath->outerjoinpath);
-				CHILD_PATH_IS_REPARAMETERIZABLE(jpath->innerjoinpath);
-			}
-			break;
-
-		case T_AppendPath:
-			{
-				AppendPath *apath = (AppendPath *) path;
-
-				CHILD_PATH_LIST_IS_REPARAMETERIZABLE(apath->subpaths);
-			}
-			break;
-
-		case T_MaterialPath:
-			{
-				MaterialPath *mpath = (MaterialPath *) path;
-
-				CHILD_PATH_IS_REPARAMETERIZABLE(mpath->subpath);
-			}
-			break;
-
-		case T_MemoizePath:
-			{
-				MemoizePath *mpath = (MemoizePath *) path;
-
-				CHILD_PATH_IS_REPARAMETERIZABLE(mpath->subpath);
-			}
-			break;
-
-		case T_GatherPath:
-			{
-				GatherPath *gpath = (GatherPath *) path;
-
-				CHILD_PATH_IS_REPARAMETERIZABLE(gpath->subpath);
-			}
-			break;
-
-		default:
-
-			/* We don't know how to reparameterize this path. */
-			return false;
-	}
-
+	Assert(result == path);
 	return true;
 }
 
@@ -4509,7 +4434,8 @@ do { \
 static List *
 reparameterize_pathlist_by_child(PlannerInfo *root,
 								 List *pathlist,
-								 RelOptInfo *child_rel)
+								 RelOptInfo *child_rel,
+								 bool check_only)
 {
 	ListCell   *lc;
 	List	   *result = NIL;
@@ -4517,7 +4443,7 @@ reparameterize_pathlist_by_child(PlannerInfo *root,
 	foreach(lc, pathlist)
 	{
 		Path	   *path = reparameterize_path_by_child(root, lfirst(lc),
-														child_rel);
+														child_rel, check_only);
 
 		if (path == NULL)
 		{
@@ -4529,24 +4455,4 @@ reparameterize_pathlist_by_child(PlannerInfo *root,
 	}
 
 	return result;
-}
-
-/*
- * pathlist_is_reparameterizable_by_child
- *		Helper function to check if a list of paths can be reparameterized.
- */
-static bool
-pathlist_is_reparameterizable_by_child(List *pathlist, RelOptInfo *child_rel)
-{
-	ListCell   *lc;
-
-	foreach(lc, pathlist)
-	{
-		Path	   *path = (Path *) lfirst(lc);
-
-		if (!path_is_reparameterizable_by_child(path, child_rel))
-			return false;
-	}
-
-	return true;
-}
+}
\ No newline at end of file
diff --git a/src/include/foreign/fdwapi.h b/src/include/foreign/fdwapi.h
index 996c62e305..5715394fde 100644
--- a/src/include/foreign/fdwapi.h
+++ b/src/include/foreign/fdwapi.h
@@ -181,7 +181,8 @@ typedef bool (*IsForeignScanParallelSafe_function) (PlannerInfo *root,
 													RangeTblEntry *rte);
 typedef List *(*ReparameterizeForeignPathByChild_function) (PlannerInfo *root,
 															List *fdw_private,
-															RelOptInfo *child_rel);
+															RelOptInfo *child_rel,
+															bool check_only);
 
 typedef bool (*IsForeignPathAsyncCapable_function) (ForeignPath *path);
 
diff --git a/src/include/nodes/extensible.h b/src/include/nodes/extensible.h
index 7d51c6033e..b7a8b6b089 100644
--- a/src/include/nodes/extensible.h
+++ b/src/include/nodes/extensible.h
@@ -102,7 +102,8 @@ typedef struct CustomPathMethods
 									List *custom_plans);
 	struct List *(*ReparameterizeCustomPathByChild) (PlannerInfo *root,
 													 List *custom_private,
-													 RelOptInfo *child_rel);
+													 RelOptInfo *child_rel,
+													 bool check_only);
 }			CustomPathMethods;
 
 /*
diff --git a/src/include/optimizer/pathnode.h b/src/include/optimizer/pathnode.h
index f5f8cbcfb4..b51c660c67 100644
--- a/src/include/optimizer/pathnode.h
+++ b/src/include/optimizer/pathnode.h
@@ -297,8 +297,9 @@ extern Path *reparameterize_path(PlannerInfo *root, Path *path,
 								 Relids required_outer,
 								 double loop_count);
 extern Path *reparameterize_path_by_child(PlannerInfo *root, Path *path,
-										  RelOptInfo *child_rel);
-extern bool path_is_reparameterizable_by_child(Path *path,
+										  RelOptInfo *child_rel,
+										  bool check_only);
+extern bool path_is_reparameterizable_by_child(PlannerInfo *root, Path *path,
 											   RelOptInfo *child_rel);
 
 /*
-- 
2.25.1