]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
Clean up remove_rel_from_query() after self-join elimination commit
authorRichard Guo <rguo@postgresql.org>
Mon, 20 Apr 2026 08:00:22 +0000 (17:00 +0900)
committerRichard Guo <rguo@postgresql.org>
Mon, 20 Apr 2026 08:00:22 +0000 (17:00 +0900)
The self-join elimination (SJE) commit grafted self-join removal onto
remove_rel_from_query(), which was originally written for left-join
removal only.  This resulted in several issues:

- Comments throughout remove_rel_from_query() still assumed only
  left-join removal, making the code misleading.

- ChangeVarNodesExtended was called on phv->phexpr with subst=-1
  during left-join removal, which is pointless and confusing since any
  surviving PHV shouldn't reference the removed rel.

- phinfo->ph_lateral was adjusted for left-join removal, which is
  unnecessary since the removed relid cannot appear in ph_lateral for
  outer joins.

- The comment about attr_needed reconstruction was in
  remove_rel_from_query(), but the actual rebuild is performed by the
  callers.

- EquivalenceClass processing in remove_rel_from_query() is redundant
  for self-join removal, since the caller (remove_self_join_rel)
  already handles ECs via update_eclasses().

- In remove_self_join_rel(), ChangeVarNodesExtended was called on
  root->processed_groupClause, which contains SortGroupClause nodes
  that have no Var nodes to rewrite.  The accompanying comment
  incorrectly mentioned "HAVING clause".

This patch fixes all these issues, clarifying the separation between
left-join removal and self-join elimination code paths within
remove_rel_from_query().  The resulting code is also better structured
for adding new types of join removal (such as inner-join removal) in
the future.

Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Tender Wang <tndrwang@gmail.com>
Reviewed-by: Andrei Lepikhov <lepihov@gmail.com>
Reviewed-by: wenhui qiu <qiuwenhuifx@gmail.com>
Discussion: https://postgr.es/m/CAMbWs48JC4OVqE=3gMB6se2WmRNNfMyFyYxm-09vgpm+Vwe8Hg@mail.gmail.com

src/backend/optimizer/plan/analyzejoins.c

index 12e9ed0d0c755596d1494620f71d997d946b2f2a..64874c0d8e359faddf263352007b7b0b058d12a9 100644 (file)
@@ -57,11 +57,13 @@ bool                enable_self_join_elimination;
 static bool join_is_removable(PlannerInfo *root, SpecialJoinInfo *sjinfo);
 static void remove_leftjoinrel_from_query(PlannerInfo *root, int relid,
                                                                                  SpecialJoinInfo *sjinfo);
+static void remove_rel_from_query(PlannerInfo *root, int relid,
+                                                                 int subst, SpecialJoinInfo *sjinfo,
+                                                                 Relids joinrelids);
 static void remove_rel_from_restrictinfo(RestrictInfo *rinfo,
                                                                                 int relid, int ojrelid);
 static void remove_rel_from_eclass(EquivalenceClass *ec,
-                                                                  SpecialJoinInfo *sjinfo,
-                                                                  int relid, int subst);
+                                                                  int relid, int ojrelid);
 static List *remove_rel_from_joinlist(List *joinlist, int relid, int *nremoved);
 static bool rel_supports_distinctness(PlannerInfo *root, RelOptInfo *rel);
 static bool rel_is_distinct_for(PlannerInfo *root, RelOptInfo *rel,
@@ -312,24 +314,151 @@ join_is_removable(PlannerInfo *root, SpecialJoinInfo *sjinfo)
        return false;
 }
 
+/*
+ * Remove the target relid and references to the target join from the
+ * planner's data structures, having determined that there is no need
+ * to include them in the query.
+ *
+ * We are not terribly thorough here.  We only bother to update parts of
+ * the planner's data structures that will actually be consulted later.
+ */
+static void
+remove_leftjoinrel_from_query(PlannerInfo *root, int relid,
+                                                         SpecialJoinInfo *sjinfo)
+{
+       RelOptInfo *rel = find_base_rel(root, relid);
+       int                     ojrelid = sjinfo->ojrelid;
+       Relids          joinrelids;
+       Relids          join_plus_commute;
+       List       *joininfos;
+       ListCell   *l;
+
+       /* Compute the relid set for the join we are considering */
+       joinrelids = bms_union(sjinfo->min_lefthand, sjinfo->min_righthand);
+       Assert(ojrelid != 0);
+       joinrelids = bms_add_member(joinrelids, ojrelid);
+
+       remove_rel_from_query(root, relid, -1, sjinfo, joinrelids);
+
+       /*
+        * Remove any joinquals referencing the rel from the joininfo lists.
+        *
+        * In some cases, a joinqual has to be put back after deleting its
+        * reference to the target rel.  This can occur for pseudoconstant and
+        * outerjoin-delayed quals, which can get marked as requiring the rel in
+        * order to force them to be evaluated at or above the join.  We can't
+        * just discard them, though.  Only quals that logically belonged to the
+        * outer join being discarded should be removed from the query.
+        *
+        * We might encounter a qual that is a clone of a deletable qual with some
+        * outer-join relids added (see deconstruct_distribute_oj_quals).  To
+        * ensure we get rid of such clones as well, add the relids of all OJs
+        * commutable with this one to the set we test against for
+        * pushed-down-ness.
+        */
+       join_plus_commute = bms_union(joinrelids,
+                                                                 sjinfo->commute_above_r);
+       join_plus_commute = bms_add_members(join_plus_commute,
+                                                                               sjinfo->commute_below_l);
+
+       /*
+        * We must make a copy of the rel's old joininfo list before starting the
+        * loop, because otherwise remove_join_clause_from_rels would destroy the
+        * list while we're scanning it.
+        */
+       joininfos = list_copy(rel->joininfo);
+       foreach(l, joininfos)
+       {
+               RestrictInfo *rinfo = (RestrictInfo *) lfirst(l);
+
+               remove_join_clause_from_rels(root, rinfo, rinfo->required_relids);
+
+               if (RINFO_IS_PUSHED_DOWN(rinfo, join_plus_commute))
+               {
+                       /*
+                        * There might be references to relid or ojrelid in the
+                        * RestrictInfo's relid sets, as a consequence of PHVs having had
+                        * ph_eval_at sets that include those.  We already checked above
+                        * that any such PHV is safe (and updated its ph_eval_at), so we
+                        * can just drop those references.
+                        */
+                       remove_rel_from_restrictinfo(rinfo, relid, ojrelid);
+
+                       /*
+                        * Cross-check that the clause itself does not reference the
+                        * target rel or join.
+                        */
+#ifdef USE_ASSERT_CHECKING
+                       {
+                               Relids          clause_varnos = pull_varnos(root,
+                                                                                                               (Node *) rinfo->clause);
+
+                               Assert(!bms_is_member(relid, clause_varnos));
+                               Assert(!bms_is_member(ojrelid, clause_varnos));
+                       }
+#endif
+                       /* Now throw it back into the joininfo lists */
+                       distribute_restrictinfo_to_rels(root, rinfo);
+               }
+       }
+
+       /*
+        * There may be references to the rel in root->fkey_list, but if so,
+        * match_foreign_keys_to_quals() will get rid of them.
+        */
+
+       /*
+        * Now remove the rel from the baserel array to prevent it from being
+        * referenced again.  (We can't do this earlier because
+        * remove_join_clause_from_rels will touch it.)
+        */
+       root->simple_rel_array[relid] = NULL;
+       root->simple_rte_array[relid] = NULL;
+
+       /* And nuke the RelOptInfo, just in case there's another access path */
+       pfree(rel);
+
+       /*
+        * Now repeat construction of attr_needed bits coming from all other
+        * sources.
+        */
+       rebuild_placeholder_attr_needed(root);
+       rebuild_joinclause_attr_needed(root);
+       rebuild_eclass_attr_needed(root);
+       rebuild_lateral_attr_needed(root);
+}
 
 /*
- * Remove the target rel->relid and references to the target join from the
+ * Remove the target relid and references to the target join from the
  * planner's data structures, having determined that there is no need
- * to include them in the query. Optionally replace them with subst if subst
- * is non-negative.
+ * to include them in the query.  Optionally replace references to the
+ * removed relid with subst if this is a self-join removal.
+ *
+ * This function serves as the common infrastructure for left-join removal
+ * and self-join elimination.  It is intentionally scoped to update only the
+ * shared planner data structures that are universally affected by relation
+ * removal.  Each specific caller remains responsible for updating any
+ * remaining data structures required by its unique removal logic.
  *
- * This function updates only parts needed for both left-join removal and
- * self-join removal.
+ * The specific type of removal being performed is dictated by the combination
+ * of the sjinfo and subst parameters.  A non-NULL sjinfo indicates left-join
+ * removal.  When sjinfo is NULL, a positive subst value indicates self-join
+ * elimination (where references are replaced with subst).
  */
 static void
-remove_rel_from_query(PlannerInfo *root, RelOptInfo *rel,
+remove_rel_from_query(PlannerInfo *root, int relid,
                                          int subst, SpecialJoinInfo *sjinfo,
                                          Relids joinrelids)
 {
-       int                     relid = rel->relid;
+       int                     ojrelid = sjinfo ? sjinfo->ojrelid : 0;
        Index           rti;
        ListCell   *l;
+       bool            is_outer_join = (sjinfo != NULL);
+       bool            is_self_join = (!is_outer_join && subst > 0);
+
+       Assert(is_outer_join || is_self_join);
+       Assert(!is_outer_join || ojrelid > 0);
+       Assert(!is_outer_join || joinrelids != NULL);
 
        /*
         * Update all_baserels and related relid sets.
@@ -337,21 +466,20 @@ remove_rel_from_query(PlannerInfo *root, RelOptInfo *rel,
        root->all_baserels = adjust_relid_set(root->all_baserels, relid, subst);
        root->all_query_rels = adjust_relid_set(root->all_query_rels, relid, subst);
 
-       if (sjinfo != NULL)
+       if (is_outer_join)
        {
-               root->outer_join_rels = bms_del_member(root->outer_join_rels,
-                                                                                          sjinfo->ojrelid);
-               root->all_query_rels = bms_del_member(root->all_query_rels,
-                                                                                         sjinfo->ojrelid);
+               root->outer_join_rels = bms_del_member(root->outer_join_rels, ojrelid);
+               root->all_query_rels = bms_del_member(root->all_query_rels, ojrelid);
        }
 
        /*
         * Likewise remove references from SpecialJoinInfo data structures.
         *
-        * This is relevant in case the outer join we're deleting is nested inside
-        * other outer joins: the upper joins' relid sets have to be adjusted. The
-        * RHS of the target outer join will be made empty here, but that's OK
-        * since caller will delete that SpecialJoinInfo entirely.
+        * This is relevant in case the relation we're deleting is part of the
+        * relid sets of special joins: those sets have to be adjusted.  If we are
+        * removing an outer join, the RHS of the target outer join will be made
+        * empty here, but that's OK since the caller will delete that
+        * SpecialJoinInfo entirely.
         */
        foreach(l, root->join_info_list)
        {
@@ -367,39 +495,32 @@ remove_rel_from_query(PlannerInfo *root, RelOptInfo *rel,
                sjinf->min_righthand = bms_copy(sjinf->min_righthand);
                sjinf->syn_lefthand = bms_copy(sjinf->syn_lefthand);
                sjinf->syn_righthand = bms_copy(sjinf->syn_righthand);
-               /* Now remove relid from the sets: */
+
+               /* Now adjust relid bit in the sets: */
                sjinf->min_lefthand = adjust_relid_set(sjinf->min_lefthand, relid, subst);
                sjinf->min_righthand = adjust_relid_set(sjinf->min_righthand, relid, subst);
                sjinf->syn_lefthand = adjust_relid_set(sjinf->syn_lefthand, relid, subst);
                sjinf->syn_righthand = adjust_relid_set(sjinf->syn_righthand, relid, subst);
 
-               if (sjinfo != NULL)
+               if (is_outer_join)
                {
-                       Assert(subst <= 0);
-
-                       /* Remove sjinfo->ojrelid bits from the sets: */
-                       sjinf->min_lefthand = bms_del_member(sjinf->min_lefthand,
-                                                                                                sjinfo->ojrelid);
-                       sjinf->min_righthand = bms_del_member(sjinf->min_righthand,
-                                                                                                 sjinfo->ojrelid);
-                       sjinf->syn_lefthand = bms_del_member(sjinf->syn_lefthand,
-                                                                                                sjinfo->ojrelid);
-                       sjinf->syn_righthand = bms_del_member(sjinf->syn_righthand,
-                                                                                                 sjinfo->ojrelid);
+                       /* Remove ojrelid bit from the sets: */
+                       sjinf->min_lefthand = bms_del_member(sjinf->min_lefthand, ojrelid);
+                       sjinf->min_righthand = bms_del_member(sjinf->min_righthand, ojrelid);
+                       sjinf->syn_lefthand = bms_del_member(sjinf->syn_lefthand, ojrelid);
+                       sjinf->syn_righthand = bms_del_member(sjinf->syn_righthand, ojrelid);
                        /* relid cannot appear in these fields, but ojrelid can: */
-                       sjinf->commute_above_l = bms_del_member(sjinf->commute_above_l,
-                                                                                                       sjinfo->ojrelid);
-                       sjinf->commute_above_r = bms_del_member(sjinf->commute_above_r,
-                                                                                                       sjinfo->ojrelid);
-                       sjinf->commute_below_l = bms_del_member(sjinf->commute_below_l,
-                                                                                                       sjinfo->ojrelid);
-                       sjinf->commute_below_r = bms_del_member(sjinf->commute_below_r,
-                                                                                                       sjinfo->ojrelid);
+                       sjinf->commute_above_l = bms_del_member(sjinf->commute_above_l, ojrelid);
+                       sjinf->commute_above_r = bms_del_member(sjinf->commute_above_r, ojrelid);
+                       sjinf->commute_below_l = bms_del_member(sjinf->commute_below_l, ojrelid);
+                       sjinf->commute_below_r = bms_del_member(sjinf->commute_below_r, ojrelid);
                }
                else
                {
-                       Assert(subst > 0);
-
+                       /*
+                        * For self-join removal, replace relid references in
+                        * semi_rhs_exprs.
+                        */
                        ChangeVarNodesExtended((Node *) sjinf->semi_rhs_exprs, relid, subst,
                                                                   0, replace_relid_callback);
                }
@@ -407,8 +528,8 @@ remove_rel_from_query(PlannerInfo *root, RelOptInfo *rel,
 
        /*
         * Likewise remove references from PlaceHolderVar data structures,
-        * removing any no-longer-needed placeholders entirely.  We remove PHV
-        * only for left-join removal.  With self-join elimination, PHVs already
+        * removing any no-longer-needed placeholders entirely.  We only remove
+        * PHVs for left-join removal.  With self-join elimination, PHVs already
         * get moved to the remaining relation, where they might still be needed.
         * It might also happen that we skip the removal of some PHVs that could
         * be removed.  However, the overhead of extra PHVs is small compared to
@@ -428,17 +549,13 @@ remove_rel_from_query(PlannerInfo *root, RelOptInfo *rel,
        {
                PlaceHolderInfo *phinfo = (PlaceHolderInfo *) lfirst(l);
 
-               Assert(sjinfo == NULL || !bms_is_member(relid, phinfo->ph_lateral));
-               if (sjinfo != NULL &&
+               Assert(!is_outer_join || !bms_is_member(relid, phinfo->ph_lateral));
+
+               if (is_outer_join &&
                        bms_is_subset(phinfo->ph_needed, joinrelids) &&
                        bms_is_member(relid, phinfo->ph_eval_at) &&
-                       !bms_is_member(sjinfo->ojrelid, phinfo->ph_eval_at))
+                       !bms_is_member(ojrelid, phinfo->ph_eval_at))
                {
-                       /*
-                        * This code shouldn't be executed if one relation is substituted
-                        * with another: in this case, the placeholder may be employed in
-                        * a filter inside the scan node the SJE removes.
-                        */
                        root->placeholder_list = foreach_delete_current(root->placeholder_list,
                                                                                                                        l);
                        root->placeholder_array[phinfo->phid] = NULL;
@@ -448,33 +565,42 @@ remove_rel_from_query(PlannerInfo *root, RelOptInfo *rel,
                        PlaceHolderVar *phv = phinfo->ph_var;
 
                        phinfo->ph_eval_at = adjust_relid_set(phinfo->ph_eval_at, relid, subst);
-                       if (sjinfo != NULL)
-                               phinfo->ph_eval_at = adjust_relid_set(phinfo->ph_eval_at,
-                                                                                                         sjinfo->ojrelid, subst);
+                       if (is_outer_join)
+                               phinfo->ph_eval_at = bms_del_member(phinfo->ph_eval_at, ojrelid);
                        Assert(!bms_is_empty(phinfo->ph_eval_at));      /* checked previously */
+
                        /* Reduce ph_needed to contain only "relation 0"; see below */
                        if (bms_is_member(0, phinfo->ph_needed))
                                phinfo->ph_needed = bms_make_singleton(0);
                        else
                                phinfo->ph_needed = NULL;
 
-                       phinfo->ph_lateral = adjust_relid_set(phinfo->ph_lateral, relid, subst);
+                       phv->phrels = adjust_relid_set(phv->phrels, relid, subst);
+                       if (is_outer_join)
+                               phv->phrels = bms_del_member(phv->phrels, ojrelid);
+                       Assert(!bms_is_empty(phv->phrels));
 
                        /*
-                        * ph_lateral might contain rels mentioned in ph_eval_at after the
-                        * replacement, remove them.
+                        * For self-join removal, update Var nodes within the PHV's
+                        * expression to reference the replacement relid, and adjust
+                        * ph_lateral for the relid substitution.  (For left-join removal,
+                        * we're removing rather than replacing, and any surviving PHV
+                        * shouldn't reference the removed rel in its expression.  Also,
+                        * relid can't appear in ph_lateral for outer joins.)
                         */
-                       phinfo->ph_lateral = bms_difference(phinfo->ph_lateral, phinfo->ph_eval_at);
-                       /* ph_lateral might or might not be empty */
-
-                       phv->phrels = adjust_relid_set(phv->phrels, relid, subst);
-                       if (sjinfo != NULL)
-                               phv->phrels = adjust_relid_set(phv->phrels,
-                                                                                          sjinfo->ojrelid, subst);
-                       Assert(!bms_is_empty(phv->phrels));
+                       if (is_self_join)
+                       {
+                               ChangeVarNodesExtended((Node *) phv->phexpr, relid, subst, 0,
+                                                                          replace_relid_callback);
+                               phinfo->ph_lateral = adjust_relid_set(phinfo->ph_lateral, relid, subst);
 
-                       ChangeVarNodesExtended((Node *) phv->phexpr, relid, subst, 0,
-                                                                  replace_relid_callback);
+                               /*
+                                * ph_lateral might contain rels mentioned in ph_eval_at after
+                                * the replacement, remove them.
+                                */
+                               phinfo->ph_lateral = bms_difference(phinfo->ph_lateral, phinfo->ph_eval_at);
+                               /* ph_lateral might or might not be empty */
+                       }
 
                        Assert(phv->phnullingrels == NULL); /* no need to adjust */
                }
@@ -482,29 +608,40 @@ remove_rel_from_query(PlannerInfo *root, RelOptInfo *rel,
 
        /*
         * Likewise remove references from EquivalenceClasses.
+        *
+        * For self-join removal, the caller has already updated the
+        * EquivalenceClasses, so we can skip this step.
         */
-       foreach(l, root->eq_classes)
+       if (is_outer_join)
        {
-               EquivalenceClass *ec = (EquivalenceClass *) lfirst(l);
+               foreach(l, root->eq_classes)
+               {
+                       EquivalenceClass *ec = (EquivalenceClass *) lfirst(l);
 
-               if (bms_is_member(relid, ec->ec_relids) ||
-                       (sjinfo == NULL || bms_is_member(sjinfo->ojrelid, ec->ec_relids)))
-                       remove_rel_from_eclass(ec, sjinfo, relid, subst);
+                       if (bms_is_member(relid, ec->ec_relids) ||
+                               bms_is_member(ojrelid, ec->ec_relids))
+                               remove_rel_from_eclass(ec, relid, ojrelid);
+               }
        }
 
        /*
-        * Finally, we must recompute per-Var attr_needed and per-PlaceHolderVar
-        * ph_needed relid sets.  These have to be known accurately, else we may
-        * fail to remove other now-removable outer joins.  And our removal of the
-        * join clause(s) for this outer join may mean that Vars that were
-        * formerly needed no longer are.  So we have to do this honestly by
-        * repeating the construction of those relid sets.  We can cheat to one
-        * small extent: we can avoid re-examining the targetlist and HAVING qual
-        * by preserving "relation 0" bits from the existing relid sets.  This is
-        * safe because we'd never remove such references.
+        * Finally, we must prepare for the caller to recompute per-Var
+        * attr_needed and per-PlaceHolderVar ph_needed relid sets.  These have to
+        * be known accurately, else we may fail to remove other now-removable
+        * joins.  Because the caller removes the join clause(s) associated with
+        * the removed join, Vars that were formerly needed may no longer be.
         *
-        * So, start by removing all other bits from attr_needed sets and
-        * lateral_vars lists.  (We already did this above for ph_needed.)
+        * The actual reconstruction of these relid sets is performed by the
+        * specific caller.  Here, we simply clear out the existing attr_needed
+        * sets (we already did this above for ph_needed) to ensure they are
+        * rebuilt from scratch.  We can cheat to one small extent: we can avoid
+        * re-examining the targetlist and HAVING qual by preserving "relation 0"
+        * bits from the existing relid sets.  This is safe because we'd never
+        * remove such references.
+        *
+        * Additionally, if we are performing self-join elimination, we must
+        * replace references to the removed relid with subst within the
+        * lateral_vars lists.
         */
        for (rti = 1; rti < root->simple_rel_array_size; rti++)
        {
@@ -527,126 +664,12 @@ remove_rel_from_query(PlannerInfo *root, RelOptInfo *rel,
                                otherrel->attr_needed[attroff] = NULL;
                }
 
-               if (subst > 0)
+               if (is_self_join)
                        ChangeVarNodesExtended((Node *) otherrel->lateral_vars, relid,
                                                                   subst, 0, replace_relid_callback);
        }
 }
 
-/*
- * Remove the target relid and references to the target join from the
- * planner's data structures, having determined that there is no need
- * to include them in the query.
- *
- * We are not terribly thorough here.  We only bother to update parts of
- * the planner's data structures that will actually be consulted later.
- */
-static void
-remove_leftjoinrel_from_query(PlannerInfo *root, int relid,
-                                                         SpecialJoinInfo *sjinfo)
-{
-       RelOptInfo *rel = find_base_rel(root, relid);
-       int                     ojrelid = sjinfo->ojrelid;
-       Relids          joinrelids;
-       Relids          join_plus_commute;
-       List       *joininfos;
-       ListCell   *l;
-
-       /* Compute the relid set for the join we are considering */
-       joinrelids = bms_union(sjinfo->min_lefthand, sjinfo->min_righthand);
-       Assert(ojrelid != 0);
-       joinrelids = bms_add_member(joinrelids, ojrelid);
-
-       remove_rel_from_query(root, rel, -1, sjinfo, joinrelids);
-
-       /*
-        * Remove any joinquals referencing the rel from the joininfo lists.
-        *
-        * In some cases, a joinqual has to be put back after deleting its
-        * reference to the target rel.  This can occur for pseudoconstant and
-        * outerjoin-delayed quals, which can get marked as requiring the rel in
-        * order to force them to be evaluated at or above the join.  We can't
-        * just discard them, though.  Only quals that logically belonged to the
-        * outer join being discarded should be removed from the query.
-        *
-        * We might encounter a qual that is a clone of a deletable qual with some
-        * outer-join relids added (see deconstruct_distribute_oj_quals).  To
-        * ensure we get rid of such clones as well, add the relids of all OJs
-        * commutable with this one to the set we test against for
-        * pushed-down-ness.
-        */
-       join_plus_commute = bms_union(joinrelids,
-                                                                 sjinfo->commute_above_r);
-       join_plus_commute = bms_add_members(join_plus_commute,
-                                                                               sjinfo->commute_below_l);
-
-       /*
-        * We must make a copy of the rel's old joininfo list before starting the
-        * loop, because otherwise remove_join_clause_from_rels would destroy the
-        * list while we're scanning it.
-        */
-       joininfos = list_copy(rel->joininfo);
-       foreach(l, joininfos)
-       {
-               RestrictInfo *rinfo = (RestrictInfo *) lfirst(l);
-
-               remove_join_clause_from_rels(root, rinfo, rinfo->required_relids);
-
-               if (RINFO_IS_PUSHED_DOWN(rinfo, join_plus_commute))
-               {
-                       /*
-                        * There might be references to relid or ojrelid in the
-                        * RestrictInfo's relid sets, as a consequence of PHVs having had
-                        * ph_eval_at sets that include those.  We already checked above
-                        * that any such PHV is safe (and updated its ph_eval_at), so we
-                        * can just drop those references.
-                        */
-                       remove_rel_from_restrictinfo(rinfo, relid, ojrelid);
-
-                       /*
-                        * Cross-check that the clause itself does not reference the
-                        * target rel or join.
-                        */
-#ifdef USE_ASSERT_CHECKING
-                       {
-                               Relids          clause_varnos = pull_varnos(root,
-                                                                                                               (Node *) rinfo->clause);
-
-                               Assert(!bms_is_member(relid, clause_varnos));
-                               Assert(!bms_is_member(ojrelid, clause_varnos));
-                       }
-#endif
-                       /* Now throw it back into the joininfo lists */
-                       distribute_restrictinfo_to_rels(root, rinfo);
-               }
-       }
-
-       /*
-        * There may be references to the rel in root->fkey_list, but if so,
-        * match_foreign_keys_to_quals() will get rid of them.
-        */
-
-       /*
-        * Now remove the rel from the baserel array to prevent it from being
-        * referenced again.  (We can't do this earlier because
-        * remove_join_clause_from_rels will touch it.)
-        */
-       root->simple_rel_array[relid] = NULL;
-       root->simple_rte_array[relid] = NULL;
-
-       /* And nuke the RelOptInfo, just in case there's another access path */
-       pfree(rel);
-
-       /*
-        * Now repeat construction of attr_needed bits coming from all other
-        * sources.
-        */
-       rebuild_placeholder_attr_needed(root);
-       rebuild_joinclause_attr_needed(root);
-       rebuild_eclass_attr_needed(root);
-       rebuild_lateral_attr_needed(root);
-}
-
 /*
  * Remove any references to relid or ojrelid from the RestrictInfo.
  *
@@ -707,8 +730,7 @@ remove_rel_from_restrictinfo(RestrictInfo *rinfo, int relid, int ojrelid)
 }
 
 /*
- * Remove any references to relid or sjinfo->ojrelid (if sjinfo != NULL)
- * from the EquivalenceClass.
+ * Remove any references to relid or ojrelid from the EquivalenceClass.
  *
  * Like remove_rel_from_restrictinfo, we don't worry about cleaning out
  * any nullingrel bits in contained Vars and PHVs.  (This might have to be
@@ -717,16 +739,13 @@ remove_rel_from_restrictinfo(RestrictInfo *rinfo, int relid, int ojrelid)
  * level(s).
  */
 static void
-remove_rel_from_eclass(EquivalenceClass *ec, SpecialJoinInfo *sjinfo,
-                                          int relid, int subst)
+remove_rel_from_eclass(EquivalenceClass *ec, int relid, int ojrelid)
 {
        ListCell   *lc;
 
        /* Fix up the EC's overall relids */
-       ec->ec_relids = adjust_relid_set(ec->ec_relids, relid, subst);
-       if (sjinfo != NULL)
-               ec->ec_relids = adjust_relid_set(ec->ec_relids,
-                                                                                sjinfo->ojrelid, subst);
+       ec->ec_relids = bms_del_member(ec->ec_relids, relid);
+       ec->ec_relids = bms_del_member(ec->ec_relids, ojrelid);
 
        /*
         * We don't expect any EC child members to exist at this point.  Ensure
@@ -745,14 +764,11 @@ remove_rel_from_eclass(EquivalenceClass *ec, SpecialJoinInfo *sjinfo,
                EquivalenceMember *cur_em = (EquivalenceMember *) lfirst(lc);
 
                if (bms_is_member(relid, cur_em->em_relids) ||
-                       (sjinfo != NULL && bms_is_member(sjinfo->ojrelid,
-                                                                                        cur_em->em_relids)))
+                       bms_is_member(ojrelid, cur_em->em_relids))
                {
                        Assert(!cur_em->em_is_const);
-                       cur_em->em_relids = adjust_relid_set(cur_em->em_relids, relid, subst);
-                       if (sjinfo != NULL)
-                               cur_em->em_relids = adjust_relid_set(cur_em->em_relids,
-                                                                                                        sjinfo->ojrelid, subst);
+                       cur_em->em_relids = bms_del_member(cur_em->em_relids, relid);
+                       cur_em->em_relids = bms_del_member(cur_em->em_relids, ojrelid);
                        if (bms_is_empty(cur_em->em_relids))
                                ec->ec_members = foreach_delete_current(ec->ec_members, lc);
                }
@@ -763,11 +779,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);
-               else
-                       remove_rel_from_restrictinfo(rinfo, relid, sjinfo->ojrelid);
+               remove_rel_from_restrictinfo(rinfo, relid, ojrelid);
        }
 
        /*
@@ -1958,14 +1970,11 @@ remove_self_join_rel(PlannerInfo *root, PlanRowMark *kmark, PlanRowMark *rmark,
                                                   0, replace_relid_callback);
 
        /* Replace links in the planner info */
-       remove_rel_from_query(root, toRemove, toKeep->relid, NULL, NULL);
+       remove_rel_from_query(root, toRemove->relid, toKeep->relid, NULL, NULL);
 
-       /* At last, replace varno in root targetlist and HAVING clause */
+       /* Replace varno in the fully-processed targetlist */
        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);
 
        adjust_relid_set(root->all_result_relids, toRemove->relid, toKeep->relid);
        adjust_relid_set(root->leaf_result_relids, toRemove->relid, toKeep->relid);