v0-0001-Switch-the-approach-to-ChangeVarNodes-s-extensibi.patch

From 6b68703d7b38326393da58e42618e4915a0e4590 Mon Sep 17 00:00:00 2001
From: "Andrei V. Lepikhov" <lepihov@gmail.com>
Date: Fri, 11 Apr 2025 14:30:33 +0200
Subject: [PATCH v0] Switch the approach to ChangeVarNodes's extensibility.

---
 src/backend/optimizer/plan/analyzejoins.c | 149 +++++++++++++++++++---
 src/backend/rewrite/rewriteManip.c        |  95 ++------------
 src/include/rewrite/rewriteManip.h        |  14 +-
 3 files changed, 161 insertions(+), 97 deletions(-)

diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c
index 6b58567f511..0f0ed1785e6 100644
--- a/src/backend/optimizer/plan/analyzejoins.c
+++ b/src/backend/optimizer/plan/analyzejoins.c
@@ -74,6 +74,7 @@ static bool is_innerrel_unique_for(PlannerInfo *root,
 								   List *restrictlist,
 								   List **extra_clauses);
 static int	self_join_candidates_cmp(const void *a, const void *b);
+static bool ChangeVarNodes_callback(Node *node, void *arg);
 
 
 /*
@@ -397,7 +398,8 @@ remove_rel_from_query(PlannerInfo *root, RelOptInfo *rel,
 		{
 			Assert(subst > 0);
 
-			ChangeVarNodes((Node *) sjinf->semi_rhs_exprs, relid, subst, 0);
+			ChangeVarNodesExtended((Node *) sjinf->semi_rhs_exprs, relid, subst,
+									0, ChangeVarNodes_callback);
 		}
 	}
 
@@ -458,7 +460,8 @@ remove_rel_from_query(PlannerInfo *root, RelOptInfo *rel,
 											   sjinfo->ojrelid, subst);
 			Assert(!bms_is_empty(phv->phrels));
 
-			ChangeVarNodes((Node *) phv->phexpr, relid, subst, 0);
+			ChangeVarNodesExtended((Node *) phv->phexpr, relid, subst, 0,
+									ChangeVarNodes_callback);
 
 			Assert(phv->phnullingrels == NULL); /* no need to adjust */
 		}
@@ -512,7 +515,8 @@ remove_rel_from_query(PlannerInfo *root, RelOptInfo *rel,
 		}
 
 		if (subst > 0)
-			ChangeVarNodes((Node *) otherrel->lateral_vars, relid, subst, 0);
+			ChangeVarNodesExtended((Node *) otherrel->lateral_vars, relid,
+									subst, 0, ChangeVarNodes_callback);
 	}
 }
 
@@ -746,7 +750,8 @@ remove_rel_from_eclass(EquivalenceClass *ec, SpecialJoinInfo *sjinfo,
 		RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc);
 
 		if (sjinfo == NULL)
-			ChangeVarNodes((Node *) rinfo, relid, subst, 0);
+			ChangeVarNodesExtended((Node *) rinfo, relid, subst, 0,
+									ChangeVarNodes_callback);
 		else
 			remove_rel_from_restrictinfo(rinfo, relid, sjinfo->ojrelid);
 	}
@@ -1537,7 +1542,8 @@ update_eclasses(EquivalenceClass *ec, int from, int to)
 		em->em_jdomain->jd_relids = adjust_relid_set(em->em_jdomain->jd_relids, from, to);
 
 		/* We only process inner joins */
-		ChangeVarNodes((Node *) em->em_expr, from, to, 0);
+		ChangeVarNodesExtended((Node *) em->em_expr, from, to, 0,
+								ChangeVarNodes_callback);
 
 		foreach_node(EquivalenceMember, other, new_members)
 		{
@@ -1571,7 +1577,8 @@ update_eclasses(EquivalenceClass *ec, int from, int to)
 			continue;
 		}
 
-		ChangeVarNodes((Node *) rinfo, from, to, 0);
+		ChangeVarNodesExtended((Node *) rinfo, from, to, 0,
+								ChangeVarNodes_callback);
 
 		/*
 		 * After switching the clause to the remaining relation, check it for
@@ -1666,6 +1673,108 @@ add_non_redundant_clauses(PlannerInfo *root,
 	}
 }
 
+static bool
+ChangeVarNodes_callback(Node *node, void *arg)
+{
+	ChangeVarNodes_context *context = (ChangeVarNodes_context *) arg;
+
+	if (node == NULL)
+		return false;
+
+	if (IsA(node, RangeTblRef))
+	{
+		return false;
+	}
+	else if (IsA(node, RestrictInfo))
+	{
+		RestrictInfo   *rinfo = (RestrictInfo *) node;
+		int				relid = -1;
+		bool			is_req_equal =
+							(rinfo->required_relids == rinfo->clause_relids);
+		bool			is_multiple =
+						(bms_membership(rinfo->clause_relids) == BMS_MULTIPLE);
+
+		if (bms_is_member(context->rt_index,
+					bms_union(rinfo->clause_relids, rinfo->required_relids)))
+		{
+			Relids new_clause_relids;
+
+			expression_tree_walker((Node *) rinfo->clause,
+									ChangeVarNodes_callback, arg);
+			expression_tree_walker((Node *) rinfo->orclause,
+									ChangeVarNodes_callback, arg);
+
+			new_clause_relids = adjust_relid_set(rinfo->clause_relids,
+												 context->rt_index,
+												 context->new_index);
+
+			/* This operation is legal until we remove only baserels */
+			rinfo->num_base_rels -= bms_num_members(rinfo->clause_relids) -
+											bms_num_members(new_clause_relids);
+
+			rinfo->clause_relids = new_clause_relids;
+			rinfo->num_base_rels = bms_num_members(rinfo->clause_relids);
+			rinfo->left_relids = adjust_relid_set(rinfo->left_relids,
+												  context->rt_index,
+												  context->new_index);
+			rinfo->right_relids = adjust_relid_set(rinfo->right_relids,
+												   context->rt_index,
+												   context->new_index);
+		}
+
+		if (is_req_equal)
+			rinfo->required_relids = rinfo->clause_relids;
+		else
+			rinfo->required_relids = adjust_relid_set(rinfo->required_relids,
+													  context->rt_index,
+													  context->new_index);
+
+		rinfo->outer_relids = adjust_relid_set(rinfo->outer_relids,
+											   context->rt_index,
+											   context->new_index);
+		rinfo->incompatible_relids = adjust_relid_set(rinfo->incompatible_relids,
+													  context->rt_index,
+													  context->new_index);
+
+		if (rinfo->mergeopfamilies &&
+			bms_get_singleton_member(rinfo->clause_relids, &relid) &&
+			is_multiple &&
+			relid == context->new_index && IsA(rinfo->clause, OpExpr))
+		{
+			Expr	   *leftOp;
+			Expr	   *rightOp;
+
+			leftOp = (Expr *) get_leftop(rinfo->clause);
+			rightOp = (Expr *) get_rightop(rinfo->clause);
+
+			/*
+			 * For self-join elimination, changing varnos could transform
+			 * "t1.a = t2.a" into "t1.a = t1.a".  That is always true as long
+			 * as "t1.a" is not null.  We use qual() to check for such a case,
+			 * and then we replace the qual for a check for not null
+			 * (NullTest).
+			 */
+			if (leftOp != NULL && equal(leftOp, rightOp))
+			{
+				NullTest   *ntest = makeNode(NullTest);
+
+				ntest->arg = leftOp;
+				ntest->nulltesttype = IS_NOT_NULL;
+				ntest->argisrow = false;
+				ntest->location = -1;
+				rinfo->clause = (Expr *) ntest;
+				rinfo->mergeopfamilies = NIL;
+				rinfo->left_em = NULL;
+				rinfo->right_em = NULL;
+			}
+			Assert(rinfo->orclause == NULL);
+		}
+		return false;
+	}
+	else
+		return standard_ChangeVarNodes_walker(node, context);
+}
+
 /*
  * Remove a relation after we have proven that it participates only in an
  * unneeded unique self-join.
@@ -1714,7 +1823,8 @@ remove_self_join_rel(PlannerInfo *root, PlanRowMark *kmark, PlanRowMark *rmark,
 	foreach_node(RestrictInfo, rinfo, joininfos)
 	{
 		remove_join_clause_from_rels(root, rinfo, rinfo->required_relids);
-		ChangeVarNodes((Node *) rinfo, toRemove->relid, toKeep->relid, 0);
+		ChangeVarNodesExtended((Node *) rinfo, toRemove->relid, toKeep->relid,
+								0, ChangeVarNodes_callback);
 
 		if (bms_membership(rinfo->required_relids) == BMS_MULTIPLE)
 			jinfo_candidates = lappend(jinfo_candidates, rinfo);
@@ -1732,7 +1842,8 @@ remove_self_join_rel(PlannerInfo *root, PlanRowMark *kmark, PlanRowMark *rmark,
 											 restrictlist);
 	foreach_node(RestrictInfo, rinfo, toRemove->baserestrictinfo)
 	{
-		ChangeVarNodes((Node *) rinfo, toRemove->relid, toKeep->relid, 0);
+		ChangeVarNodesExtended((Node *) rinfo, toRemove->relid, toKeep->relid,
+								0, ChangeVarNodes_callback);
 
 		if (bms_membership(rinfo->required_relids) == BMS_MULTIPLE)
 			jinfo_candidates = lappend(jinfo_candidates, rinfo);
@@ -1774,7 +1885,8 @@ remove_self_join_rel(PlannerInfo *root, PlanRowMark *kmark, PlanRowMark *rmark,
 	{
 		Node	   *node = lfirst(lc);
 
-		ChangeVarNodes(node, toRemove->relid, toKeep->relid, 0);
+		ChangeVarNodesExtended(node, toRemove->relid, toKeep->relid, 0,
+							   ChangeVarNodes_callback);
 		if (!list_member(toKeep->reltarget->exprs, node))
 			toKeep->reltarget->exprs = lappend(toKeep->reltarget->exprs, node);
 	}
@@ -1818,14 +1930,18 @@ remove_self_join_rel(PlannerInfo *root, PlanRowMark *kmark, PlanRowMark *rmark,
 	 * Replace varno in all the query structures, except nodes RangeTblRef
 	 * otherwise later remove_rel_from_joinlist will yield errors.
 	 */
-	ChangeVarNodesExtended((Node *) root->parse, toRemove->relid, toKeep->relid, 0, false);
+	ChangeVarNodesExtended((Node *) root->parse, toRemove->relid, toKeep->relid,
+						   0, ChangeVarNodes_callback);
 
 	/* Replace links in the planner info */
 	remove_rel_from_query(root, toRemove, toKeep->relid, NULL, NULL);
 
 	/* At last, replace varno in root targetlist and HAVING clause */
-	ChangeVarNodes((Node *) root->processed_tlist, toRemove->relid, toKeep->relid, 0);
-	ChangeVarNodes((Node *) root->processed_groupClause, toRemove->relid, toKeep->relid, 0);
+	ChangeVarNodesExtended((Node *) root->processed_tlist, toRemove->relid,
+							toKeep->relid, 0, ChangeVarNodes_callback);
+	ChangeVarNodesExtended((Node *) root->processed_groupClause,
+							toRemove->relid, toKeep->relid, 0,
+							ChangeVarNodes_callback);
 
 	adjust_relid_set(root->all_result_relids, toRemove->relid, toKeep->relid);
 	adjust_relid_set(root->leaf_result_relids, toRemove->relid, toKeep->relid);
@@ -1908,8 +2024,10 @@ split_selfjoin_quals(PlannerInfo *root, List *joinquals, List **selfjoinquals,
 		 * when we have cast of the same var to different (but compatible)
 		 * types.
 		 */
-		ChangeVarNodes(rightexpr, bms_singleton_member(rinfo->right_relids),
-					   bms_singleton_member(rinfo->left_relids), 0);
+		ChangeVarNodesExtended(rightexpr,
+							   bms_singleton_member(rinfo->right_relids),
+							   bms_singleton_member(rinfo->left_relids), 0,
+							   ChangeVarNodes_callback);
 
 		if (equal(leftexpr, rightexpr))
 			sjoinquals = lappend(sjoinquals, rinfo);
@@ -1945,7 +2063,8 @@ match_unique_clauses(PlannerInfo *root, RelOptInfo *outer, List *uclauses,
 			   bms_is_empty(rinfo->right_relids));
 
 		clause = (Expr *) copyObject(rinfo->clause);
-		ChangeVarNodes((Node *) clause, relid, outer->relid, 0);
+		ChangeVarNodesExtended((Node *) clause, relid, outer->relid, 0,
+							   ChangeVarNodes_callback);
 
 		iclause = bms_is_empty(rinfo->left_relids) ? get_rightop(clause) :
 			get_leftop(clause);
diff --git a/src/backend/rewrite/rewriteManip.c b/src/backend/rewrite/rewriteManip.c
index 1c61085a0a7..790cd738f9e 100644
--- a/src/backend/rewrite/rewriteManip.c
+++ b/src/backend/rewrite/rewriteManip.c
@@ -550,16 +550,18 @@ offset_relid_set(Relids relids, int offset)
  * earlier to ensure that no unwanted side-effects occur!
  */
 
-typedef struct
-{
-	int			rt_index;
-	int			new_index;
-	int			sublevels_up;
-	bool		change_RangeTblRef;
-} ChangeVarNodes_context;
-
 static bool
 ChangeVarNodes_walker(Node *node, ChangeVarNodes_context *context)
+{
+	/* if caller has defined a callback it may need to alter default behaviour */
+	if (context->callback)
+		return context->callback(node, (void *) context);
+	else
+		return standard_ChangeVarNodes_walker(node, context);
+}
+
+bool
+standard_ChangeVarNodes_walker(Node *node, ChangeVarNodes_context *context)
 {
 	if (node == NULL)
 		return false;
@@ -588,7 +590,7 @@ ChangeVarNodes_walker(Node *node, ChangeVarNodes_context *context)
 			cexpr->cvarno = context->new_index;
 		return false;
 	}
-	if (IsA(node, RangeTblRef) && context->change_RangeTblRef)
+	if (IsA(node, RangeTblRef))
 	{
 		RangeTblRef *rtr = (RangeTblRef *) node;
 
@@ -635,75 +637,6 @@ ChangeVarNodes_walker(Node *node, ChangeVarNodes_context *context)
 		}
 		return false;
 	}
-	if (IsA(node, RestrictInfo))
-	{
-		RestrictInfo *rinfo = (RestrictInfo *) node;
-		int			relid = -1;
-		bool		is_req_equal =
-			(rinfo->required_relids == rinfo->clause_relids);
-		bool		clause_relids_is_multiple =
-			(bms_membership(rinfo->clause_relids) == BMS_MULTIPLE);
-
-		if (bms_is_member(context->rt_index, rinfo->clause_relids))
-		{
-			expression_tree_walker((Node *) rinfo->clause, ChangeVarNodes_walker, (void *) context);
-			expression_tree_walker((Node *) rinfo->orclause, ChangeVarNodes_walker, (void *) context);
-
-			rinfo->clause_relids =
-				adjust_relid_set(rinfo->clause_relids, context->rt_index, context->new_index);
-			rinfo->num_base_rels = bms_num_members(rinfo->clause_relids);
-			rinfo->left_relids =
-				adjust_relid_set(rinfo->left_relids, context->rt_index, context->new_index);
-			rinfo->right_relids =
-				adjust_relid_set(rinfo->right_relids, context->rt_index, context->new_index);
-		}
-
-		if (is_req_equal)
-			rinfo->required_relids = rinfo->clause_relids;
-		else
-			rinfo->required_relids =
-				adjust_relid_set(rinfo->required_relids, context->rt_index, context->new_index);
-
-		rinfo->outer_relids =
-			adjust_relid_set(rinfo->outer_relids, context->rt_index, context->new_index);
-		rinfo->incompatible_relids =
-			adjust_relid_set(rinfo->incompatible_relids, context->rt_index, context->new_index);
-
-		if (rinfo->mergeopfamilies &&
-			bms_get_singleton_member(rinfo->clause_relids, &relid) &&
-			clause_relids_is_multiple &&
-			relid == context->new_index && IsA(rinfo->clause, OpExpr))
-		{
-			Expr	   *leftOp;
-			Expr	   *rightOp;
-
-			leftOp = (Expr *) get_leftop(rinfo->clause);
-			rightOp = (Expr *) get_rightop(rinfo->clause);
-
-			/*
-			 * For self-join elimination, changing varnos could transform
-			 * "t1.a = t2.a" into "t1.a = t1.a".  That is always true as long
-			 * as "t1.a" is not null.  We use qual() to check for such a case,
-			 * and then we replace the qual for a check for not null
-			 * (NullTest).
-			 */
-			if (leftOp != NULL && equal(leftOp, rightOp))
-			{
-				NullTest   *ntest = makeNode(NullTest);
-
-				ntest->arg = leftOp;
-				ntest->nulltesttype = IS_NOT_NULL;
-				ntest->argisrow = false;
-				ntest->location = -1;
-				rinfo->clause = (Expr *) ntest;
-				rinfo->mergeopfamilies = NIL;
-				rinfo->left_em = NULL;
-				rinfo->right_em = NULL;
-			}
-			Assert(rinfo->orclause == NULL);
-		}
-		return false;
-	}
 	if (IsA(node, AppendRelInfo))
 	{
 		AppendRelInfo *appinfo = (AppendRelInfo *) node;
@@ -749,14 +682,14 @@ ChangeVarNodes_walker(Node *node, ChangeVarNodes_context *context)
  */
 void
 ChangeVarNodesExtended(Node *node, int rt_index, int new_index,
-					   int sublevels_up, bool change_RangeTblRef)
+					   int sublevels_up, ChangeVarNodes_callback_type callback)
 {
 	ChangeVarNodes_context context;
 
 	context.rt_index = rt_index;
 	context.new_index = new_index;
 	context.sublevels_up = sublevels_up;
-	context.change_RangeTblRef = change_RangeTblRef;
+	context.callback = callback;
 
 	if (node && IsA(node, Query))
 	{
@@ -792,7 +725,7 @@ ChangeVarNodesExtended(Node *node, int rt_index, int new_index,
 void
 ChangeVarNodes(Node *node, int rt_index, int new_index, int sublevels_up)
 {
-	ChangeVarNodesExtended(node, rt_index, new_index, sublevels_up, true);
+	ChangeVarNodesExtended(node, rt_index, new_index, sublevels_up, NULL);
 }
 
 /*
diff --git a/src/include/rewrite/rewriteManip.h b/src/include/rewrite/rewriteManip.h
index ea3908739c6..6d46274f1ee 100644
--- a/src/include/rewrite/rewriteManip.h
+++ b/src/include/rewrite/rewriteManip.h
@@ -41,7 +41,18 @@ typedef enum ReplaceVarsNoMatchOption
 	REPLACEVARS_SUBSTITUTE_NULL,	/* replace with a NULL Const */
 } ReplaceVarsNoMatchOption;
 
+typedef bool (*ChangeVarNodes_callback_type) (Node *node, void *arg);
 
+typedef struct
+{
+	int			rt_index;
+	int			new_index;
+	int			sublevels_up;
+	ChangeVarNodes_callback_type callback;
+} ChangeVarNodes_context;
+
+extern bool standard_ChangeVarNodes_walker(Node *node,
+										   ChangeVarNodes_context *context);
 extern Relids adjust_relid_set(Relids relids, int oldrelid, int newrelid);
 extern void CombineRangeTables(List **dst_rtable, List **dst_perminfos,
 							   List *src_rtable, List *src_perminfos);
@@ -49,7 +60,8 @@ extern void OffsetVarNodes(Node *node, int offset, int sublevels_up);
 extern void ChangeVarNodes(Node *node, int rt_index, int new_index,
 						   int sublevels_up);
 extern void ChangeVarNodesExtended(Node *node, int rt_index, int new_index,
-								   int sublevels_up, bool change_RangeTblRef);
+								   int sublevels_up,
+								   ChangeVarNodes_callback_type callback);
 extern void IncrementVarSublevelsUp(Node *node, int delta_sublevels_up,
 									int min_sublevels_up);
 extern void IncrementVarSublevelsUp_rtable(List *rtable,
-- 
2.39.5