From: Richard Guo Date: Mon, 22 Jun 2026 01:40:40 +0000 (+0900) Subject: Strip removed-relation references from PlaceHolderVars at join removal X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9a60f295bcb186a729d04e76377b7f122b2a1dd9;p=thirdparty%2Fpostgresql.git 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 PlaceHolderVars. 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. Reported-by: Alexander Kuzmenkov Author: Richard Guo Reviewed-by: Tender Wang Discussion: https://postgr.es/m/CALzhyqwryL2QywgO03VQr_237Sq3MEVgTTT2_A9G3nGT5-SRZg@mail.gmail.com Backpatch-through: 18 --- diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c index b07cb731401..f109af25d72 100644 --- a/src/backend/optimizer/plan/analyzejoins.c +++ b/src/backend/optimizer/plan/analyzejoins.c @@ -62,8 +62,10 @@ 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 Node *remove_rel_from_phvs(Node *node, int relid, int ojrelid); +static Node *remove_rel_from_phvs_mutator(Node *node, Relids removable); 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, @@ -618,9 +620,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 +642,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 +672,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 +762,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 and Consts 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) && !IsA(em->em_expr, Const)) + 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); @@ -808,6 +844,69 @@ remove_rel_from_eclass(EquivalenceClass *ec, int relid, int ojrelid) ec_clear_derived_clauses(ec); } +/* + * 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(Node *node, int relid, int ojrelid) +{ + Relids removable = bms_add_member(bms_make_singleton(relid), ojrelid); + + return remove_rel_from_phvs_mutator(node, removable); +} + +static Node * +remove_rel_from_phvs_mutator(Node *node, Relids removable) +{ + if (node == NULL) + return NULL; + if (IsA(node, PlaceHolderVar)) + { + PlaceHolderVar *phv = (PlaceHolderVar *) node; + Relids newphrels; + + /* Upper-level PlaceHolderVars should be long gone at this point */ + Assert(phv->phlevelsup == 0); + + /* Copy the PlaceHolderVar and mutate what's below ... */ + phv = (PlaceHolderVar *) + expression_tree_mutator(node, + remove_rel_from_phvs_mutator, + removable); + + /* + * ... 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, removable); + if (!bms_is_empty(newphrels)) + { + phv->phrels = newphrels; + phv->phnullingrels = bms_difference(phv->phnullingrels, + removable); + } + + return (Node *) phv; + } + return expression_tree_mutator(node, + remove_rel_from_phvs_mutator, + removable); +} + /* * Remove any occurrences of the target relid from a joinlist structure. * 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);