From 8c272bd45092563ec789384bc3ea9132fa61f41b Mon Sep 17 00:00:00 2001 From: Richard Guo Date: Tue, 16 Jun 2026 16:08:24 +0900 Subject: [PATCH v1] Strip removed-relation references from PlaceHolderVars at join removal When left-join removal deletes a relation, remove_rel_from_query() updates the relid sets attached to RestrictInfos and EquivalenceMembers, and the canonical PlaceHolderVar held in each PlaceHolderInfo, but it does not rewrite the PlaceHolderVars embedded in clause and EquivalenceClass member expressions. That has been fine, because later processing consults those relid sets rather than the embedded PlaceHolderVars. However, such an expression may afterwards be translated for an appendrel child and have its relids recomputed from scratch by pull_varnos(). If the embedded PlaceHolderVar's phrels still mentions the removed relation, pull_varnos() folds it back in, so the rebuilt clause's relids reference a no-longer-existent relation. That yields a parameterized path keyed on the removed relation, tripping the Assert on root->outer_join_rels in get_eclass_indexes_for_relids(). Fix by stripping the removed relids from the PlaceHolderVars in surviving rels' baserestrictinfo and in EquivalenceClass member expressions, keeping them consistent with the canonical PlaceHolderVar. This is only reachable on v18 and later, where match_index_to_operand() began ignoring PlaceHolderVars; before that, the wrapping PlaceHolderVar prevented the index match that exposes the stale relids. --- src/backend/optimizer/plan/analyzejoins.c | 138 ++++++++++++++++++++-- src/test/regress/expected/join.out | 22 ++++ src/test/regress/sql/join.sql | 12 ++ src/tools/pgindent/typedefs.list | 1 + 4 files changed, 163 insertions(+), 10 deletions(-) diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c index b07cb731401..8ccc82b8237 100644 --- a/src/backend/optimizer/plan/analyzejoins.c +++ b/src/backend/optimizer/plan/analyzejoins.c @@ -51,6 +51,13 @@ typedef struct Oid reloid; } SelfJoinCandidate; +/* Context for remove_rel_from_phvs() */ +typedef struct +{ + Relids removable; /* removed base relation and outer join */ + int sublevels_up; +} remove_rel_from_phvs_context; + bool enable_self_join_elimination; /* local functions */ @@ -62,7 +69,7 @@ static void remove_rel_from_query(PlannerInfo *root, int relid, Relids joinrelids); static void remove_rel_from_restrictinfo(RestrictInfo *rinfo, int relid, int ojrelid); -static void remove_rel_from_eclass(EquivalenceClass *ec, +static void remove_rel_from_eclass(PlannerInfo *root, EquivalenceClass *ec, 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); @@ -79,6 +86,7 @@ static bool is_innerrel_unique_for(PlannerInfo *root, static int self_join_candidates_cmp(const void *a, const void *b); static bool replace_relid_callback(Node *node, ChangeVarNodes_context *context); +static Node *remove_rel_from_phvs(Node *node, int relid, int ojrelid); /* @@ -428,6 +436,82 @@ remove_leftjoinrel_from_query(PlannerInfo *root, int relid, rebuild_lateral_attr_needed(root); } +/* + * Remove any references to the specified RT index(es) from the phrels (and + * phnullingrels) of every PlaceHolderVar in the given expression. + * + * remove_rel_from_query() fixes up the relid sets of RestrictInfos and + * EquivalenceMembers, but not the PlaceHolderVars embedded in their + * expressions. That's normally fine, but such an expression may later be + * translated for an appendrel child and have its relids recomputed by + * pull_varnos(). A leftover removed relid in phrels would then make + * pull_varnos() reference a nonexistent rel, so we strip it here to match the + * canonical PlaceHolderVar. + */ +static Node * +remove_rel_from_phvs_mutator(Node *node, remove_rel_from_phvs_context *context) +{ + if (node == NULL) + return NULL; + if (IsA(node, PlaceHolderVar)) + { + PlaceHolderVar *phv = (PlaceHolderVar *) node; + + if (phv->phlevelsup == context->sublevels_up) + { + Relids newphrels; + + /* Copy the PlaceHolderVar and mutate what's below ... */ + phv = (PlaceHolderVar *) + expression_tree_mutator(node, remove_rel_from_phvs_mutator, + context); + + /* + * ... then strip the removed rels from its relid sets. + * + * If stripping would empty phrels, the PHV is evaluated only at + * the removed relation(s); it then belongs to an + * EquivalenceMember that the caller drops immediately afterwards. + * Leave such a PHV untouched rather than build one with empty + * phrels, which the rest of the planner assumes never occurs. + */ + newphrels = bms_difference(phv->phrels, context->removable); + if (!bms_is_empty(newphrels)) + { + phv->phrels = newphrels; + phv->phnullingrels = bms_difference(phv->phnullingrels, + context->removable); + } + + return (Node *) phv; + } + } + else if (IsA(node, Query)) + { + Query *newnode; + + context->sublevels_up++; + newnode = query_tree_mutator((Query *) node, + remove_rel_from_phvs_mutator, + context, 0); + context->sublevels_up--; + return (Node *) newnode; + } + return expression_tree_mutator(node, remove_rel_from_phvs_mutator, context); +} + +static Node * +remove_rel_from_phvs(Node *node, int relid, int ojrelid) +{ + remove_rel_from_phvs_context context; + + context.removable = bms_add_member(bms_make_singleton(relid), ojrelid); + context.sublevels_up = 0; + return query_or_expression_tree_mutator(node, + remove_rel_from_phvs_mutator, + &context, 0); +} + /* * Remove the target relid and references to the target join from the * planner's data structures, having determined that there is no need @@ -618,9 +702,7 @@ remove_rel_from_query(PlannerInfo *root, int relid, { EquivalenceClass *ec = (EquivalenceClass *) lfirst(l); - if (bms_is_member(relid, ec->ec_relids) || - bms_is_member(ojrelid, ec->ec_relids)) - remove_rel_from_eclass(ec, relid, ojrelid); + remove_rel_from_eclass(root, ec, relid, ojrelid); } } @@ -642,6 +724,11 @@ remove_rel_from_query(PlannerInfo *root, int relid, * Additionally, if we are performing self-join elimination, we must * replace references to the removed relid with subst within the * lateral_vars lists. + * + * Also, for left-join removal, we strip the removed rel and join from any + * PlaceHolderVar embedded in the surviving rels' restriction clauses (see + * remove_rel_from_phvs); we needn't bother with the rel being removed, + * nor when the query has no PlaceHolderVars. */ for (rti = 1; rti < root->simple_rel_array_size; rti++) { @@ -667,6 +754,15 @@ remove_rel_from_query(PlannerInfo *root, int relid, if (is_self_join) ChangeVarNodesExtended((Node *) otherrel->lateral_vars, relid, subst, 0, replace_relid_callback); + + if (is_outer_join && rti != relid && root->glob->lastPHId != 0) + { + foreach_node(RestrictInfo, rinfo, otherrel->baserestrictinfo) + { + rinfo->clause = (Expr *) + remove_rel_from_phvs((Node *) rinfo->clause, relid, ojrelid); + } + } } } @@ -748,17 +844,39 @@ remove_rel_from_restrictinfo(RestrictInfo *rinfo, int relid, int ojrelid) /* * 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 - * improved sometime.) We do need to fix the EC and EM relid sets to ensure - * that implied join equalities will be generated at the appropriate join - * level(s). + * We fix the EC and EM relid sets to ensure that implied join equalities will + * be generated at the appropriate join level(s). We also strip the removed + * rel from PlaceHolderVars embedded in member expressions; a member's + * em_relids reflects ph_eval_at rather than the PHV's phrels, so the latter + * can still mention the removed rel even when em_relids does not. Like + * remove_rel_from_restrictinfo, we don't bother with nullingrel bits in + * contained plain Vars. */ static void -remove_rel_from_eclass(EquivalenceClass *ec, int relid, int ojrelid) +remove_rel_from_eclass(PlannerInfo *root, EquivalenceClass *ec, + int relid, int ojrelid) { ListCell *lc; + /* + * Strip the removed rel/join from PlaceHolderVars in member expressions. + * This is needed even when the EC's relids don't mention the removed rel. + * Plain Vars can't contain a PlaceHolderVar, so skip them. + */ + if (root->glob->lastPHId != 0) + { + foreach_node(EquivalenceMember, em, ec->ec_members) + { + if (!IsA(em->em_expr, Var)) + em->em_expr = (Expr *) + remove_rel_from_phvs((Node *) em->em_expr, relid, ojrelid); + } + } + + if (!bms_is_member(relid, ec->ec_relids) && + !bms_is_member(ojrelid, ec->ec_relids)) + return; + /* Fix up the EC's overall relids */ ec->ec_relids = bms_del_member(ec->ec_relids, relid); ec->ec_relids = bms_del_member(ec->ec_relids, ojrelid); diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index 78bf022f7b4..ed946abed7f 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -6564,6 +6564,28 @@ select a.* from a left join parted_b pb on a.b_id = pb.id; Seq Scan on a (1 row) +-- test that clauses that still embed PHVs are not referencing the removed +-- relation when rebuilt for a partition of the kept relation +explain (costs off) +select 1 from (select t1.id from parted_b t1 left join parted_b t2 on t1.id = t2.id) s +where s.id = 1 group by (); + QUERY PLAN +----------------------- + Result + Replaces: Aggregate +(2 rows) + +explain (costs off) +select 1 from parted_b t1 + join (select t2.id from parted_b t2 left join parted_b t3 on t2.id = t3.id) s + on t1.id = s.id +group by (); + QUERY PLAN +----------------------- + Result + Replaces: Aggregate +(2 rows) + rollback; create temp table parent (k int primary key, pd int); create temp table child (k int unique, cd int); diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql index fae19113cef..78f7b4f544d 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -2420,6 +2420,18 @@ CREATE TEMP TABLE parted_b1 partition of parted_b for values from (0) to (10); explain (costs off) select a.* from a left join parted_b pb on a.b_id = pb.id; +-- test that clauses that still embed PHVs are not referencing the removed +-- relation when rebuilt for a partition of the kept relation +explain (costs off) +select 1 from (select t1.id from parted_b t1 left join parted_b t2 on t1.id = t2.id) s +where s.id = 1 group by (); + +explain (costs off) +select 1 from parted_b t1 + join (select t2.id from parted_b t2 left join parted_b t3 on t2.id = t3.id) s + on t1.id = s.id +group by (); + rollback; create temp table parent (k int primary key, pd int); diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index f9eb23e52c9..4a8842b7ea0 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -4249,6 +4249,7 @@ remoteConn remoteConnHashEnt remoteDep remove_nulling_relids_context +remove_rel_from_phvs_context rendezvousHashEntry rep replace_rte_variables_callback -- 2.39.5 (Apple Git-154)