Revert "Refactor ChangeVarNodesExtended() using the custom callback"

This reverts commit 250a718aadad68793e82103282247556a46a3cfc.
It shouldn't be pushed during the release freeze.

Reported-by: Tom Lane
Discussion: https://postgr.es/m/E1uBIbY-000owH-0O%40gemulon.postgresql.org

1 files changed, 15 insertions, 145 deletions

diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c
index 4d55c2ea591..be19167e4a2 100644
--- a/src/backend/optimizer/plan/analyzejoins.c
+++ b/src/backend/optimizer/plan/analyzejoins.c
@@ -74,8 +74,6 @@ 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 replace_relid_callback(Node *node,
-								   ChangeVarNodes_context *context);
 
 
 /*
@@ -399,8 +397,7 @@ remove_rel_from_query(PlannerInfo *root, RelOptInfo *rel,
 		{
 			Assert(subst > 0);
 
-			ChangeVarNodesExtended((Node *) sjinf->semi_rhs_exprs, relid, subst,
-								   0, replace_relid_callback);
+			ChangeVarNodes((Node *) sjinf->semi_rhs_exprs, relid, subst, 0);
 		}
 	}
 
@@ -472,8 +469,7 @@ remove_rel_from_query(PlannerInfo *root, RelOptInfo *rel,
 											   sjinfo->ojrelid, subst);
 			Assert(!bms_is_empty(phv->phrels));
 
-			ChangeVarNodesExtended((Node *) phv->phexpr, relid, subst, 0,
-								   replace_relid_callback);
+			ChangeVarNodes((Node *) phv->phexpr, relid, subst, 0);
 
 			Assert(phv->phnullingrels == NULL); /* no need to adjust */
 		}
@@ -527,8 +523,7 @@ remove_rel_from_query(PlannerInfo *root, RelOptInfo *rel,
 		}
 
 		if (subst > 0)
-			ChangeVarNodesExtended((Node *) otherrel->lateral_vars, relid,
-								   subst, 0, replace_relid_callback);
+			ChangeVarNodes((Node *) otherrel->lateral_vars, relid, subst, 0);
 	}
 }
 
@@ -762,8 +757,7 @@ remove_rel_from_eclass(EquivalenceClass *ec, SpecialJoinInfo *sjinfo,
 		RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc);
 
 		if (sjinfo == NULL)
-			ChangeVarNodesExtended((Node *) rinfo, relid, subst, 0,
-								   replace_relid_callback);
+			ChangeVarNodes((Node *) rinfo, relid, subst, 0);
 		else
 			remove_rel_from_restrictinfo(rinfo, relid, sjinfo->ojrelid);
 	}
@@ -1554,8 +1548,7 @@ 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 */
-		ChangeVarNodesExtended((Node *) em->em_expr, from, to, 0,
-							   replace_relid_callback);
+		ChangeVarNodes((Node *) em->em_expr, from, to, 0);
 
 		foreach_node(EquivalenceMember, other, new_members)
 		{
@@ -1589,8 +1582,7 @@ update_eclasses(EquivalenceClass *ec, int from, int to)
 			continue;
 		}
 
-		ChangeVarNodesExtended((Node *) rinfo, from, to, 0,
-							   replace_relid_callback);
+		ChangeVarNodes((Node *) rinfo, from, to, 0);
 
 		/*
 		 * After switching the clause to the remaining relation, check it for
@@ -1686,118 +1678,6 @@ add_non_redundant_clauses(PlannerInfo *root,
 }
 
 /*
- * A custom callback for ChangeVarNodesExtended() providing
- * Self-join elimination (SJE) related functionality
- *
- * SJE needs to skip the RangeTblRef node
- * type.  During SJE's last step, remove_rel_from_joinlist() removes
- * remaining RangeTblRefs with target relid.  If ChangeVarNodes() replaces
- * the target relid before, remove_rel_from_joinlist() fails to identify
- * the nodes to delete.
- *
- * SJE also needs to change the relids within RestrictInfo's.
- */
-static bool
-replace_relid_callback(Node *node, ChangeVarNodes_context *context)
-{
-	if (IsA(node, RangeTblRef))
-	{
-		return true;
-	}
-	else 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);
-
-		/*
-		 * Recurse down into clauses if the target relation is present in
-		 * clause_relids or required_relids.  We must check required_relids
-		 * because the relation not present in clause_relids might still be
-		 * present somewhere in orclause.
-		 */
-		if (bms_is_member(context->rt_index, rinfo->clause_relids) ||
-			bms_is_member(context->rt_index, rinfo->required_relids))
-		{
-			Relids		new_clause_relids;
-
-			ChangeVarNodesWalkExpression((Node *) rinfo->clause, context);
-			ChangeVarNodesWalkExpression((Node *) rinfo->orclause, context);
-
-			new_clause_relids = adjust_relid_set(rinfo->clause_relids,
-												 context->rt_index,
-												 context->new_index);
-
-			/*
-			 * Incrementally adjust num_base_rels based on the change of
-			 * clause_relids, which could contain both base relids and
-			 * outer-join relids.  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->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 true;
-	}
-
-	return false;
-}
-
-/*
  * Remove a relation after we have proven that it participates only in an
  * unneeded unique self-join.
  *
@@ -1845,8 +1725,7 @@ 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);
-		ChangeVarNodesExtended((Node *) rinfo, toRemove->relid, toKeep->relid,
-							   0, replace_relid_callback);
+		ChangeVarNodes((Node *) rinfo, toRemove->relid, toKeep->relid, 0);
 
 		if (bms_membership(rinfo->required_relids) == BMS_MULTIPLE)
 			jinfo_candidates = lappend(jinfo_candidates, rinfo);
@@ -1864,8 +1743,7 @@ remove_self_join_rel(PlannerInfo *root, PlanRowMark *kmark, PlanRowMark *rmark,
 											 restrictlist);
 	foreach_node(RestrictInfo, rinfo, toRemove->baserestrictinfo)
 	{
-		ChangeVarNodesExtended((Node *) rinfo, toRemove->relid, toKeep->relid,
-							   0, replace_relid_callback);
+		ChangeVarNodes((Node *) rinfo, toRemove->relid, toKeep->relid, 0);
 
 		if (bms_membership(rinfo->required_relids) == BMS_MULTIPLE)
 			jinfo_candidates = lappend(jinfo_candidates, rinfo);
@@ -1907,8 +1785,7 @@ remove_self_join_rel(PlannerInfo *root, PlanRowMark *kmark, PlanRowMark *rmark,
 	{
 		Node	   *node = lfirst(lc);
 
-		ChangeVarNodesExtended(node, toRemove->relid, toKeep->relid, 0,
-							   replace_relid_callback);
+		ChangeVarNodes(node, toRemove->relid, toKeep->relid, 0);
 		if (!list_member(toKeep->reltarget->exprs, node))
 			toKeep->reltarget->exprs = lappend(toKeep->reltarget->exprs, node);
 	}
@@ -1952,18 +1829,14 @@ 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, replace_relid_callback);
+	ChangeVarNodesExtended((Node *) root->parse, toRemove->relid, toKeep->relid, 0, false);
 
 	/* 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 */
-	ChangeVarNodesExtended((Node *) root->processed_tlist, toRemove->relid,
-						   toKeep->relid, 0, replace_relid_callback);
-	ChangeVarNodesExtended((Node *) root->processed_groupClause,
-						   toRemove->relid, toKeep->relid, 0,
-						   replace_relid_callback);
+	ChangeVarNodes((Node *) root->processed_tlist, toRemove->relid, toKeep->relid, 0);
+	ChangeVarNodes((Node *) root->processed_groupClause, toRemove->relid, toKeep->relid, 0);
 
 	adjust_relid_set(root->all_result_relids, toRemove->relid, toKeep->relid);
 	adjust_relid_set(root->leaf_result_relids, toRemove->relid, toKeep->relid);
@@ -2046,10 +1919,8 @@ split_selfjoin_quals(PlannerInfo *root, List *joinquals, List **selfjoinquals,
 		 * when we have cast of the same var to different (but compatible)
 		 * types.
 		 */
-		ChangeVarNodesExtended(rightexpr,
-							   bms_singleton_member(rinfo->right_relids),
-							   bms_singleton_member(rinfo->left_relids), 0,
-							   replace_relid_callback);
+		ChangeVarNodes(rightexpr, bms_singleton_member(rinfo->right_relids),
+					   bms_singleton_member(rinfo->left_relids), 0);
 
 		if (equal(leftexpr, rightexpr))
 			sjoinquals = lappend(sjoinquals, rinfo);
@@ -2085,8 +1956,7 @@ match_unique_clauses(PlannerInfo *root, RelOptInfo *outer, List *uclauses,
 			   bms_is_empty(rinfo->right_relids));
 
 		clause = (Expr *) copyObject(rinfo->clause);
-		ChangeVarNodesExtended((Node *) clause, relid, outer->relid, 0,
-							   replace_relid_callback);
+		ChangeVarNodes((Node *) clause, relid, outer->relid, 0);
 
 		iclause = bms_is_empty(rinfo->left_relids) ? get_rightop(clause) :
 			get_leftop(clause);