From: Richard Guo Date: Wed, 15 Jul 2026 00:22:58 +0000 (+0900) Subject: Strip removed-relation references from PHVs in join clauses X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=18105e6db5e5314a575fdb23a99cf4809d8ef062;p=thirdparty%2Fpostgresql.git Strip removed-relation references from PHVs in join clauses Commit 9a60f295b stripped the stale PlaceHolderVars left behind by left-join removal from the surviving rels' baserestrictinfo and from EquivalenceClass member expressions, but it overlooked join clauses. A PlaceHolderVar embedded in a join clause can likewise retain the removed rel and join in its phrels, since remove_rel_from_query() fixes up the RestrictInfo's own relid sets but not the PHVs inside its expression. As before, this is normally harmless, because later processing consults those relid sets rather than the embedded PHVs. However, a restriction clause derived from such an OR join clause inherits the stale PlaceHolderVar, and when the derived clause is translated for an appendrel child, pull_varnos() recomputes its relids and folds the removed relation back in. The rebuilt clause then references a no-longer-existent relation, tripping an assertion during path generation. Fix by also stripping the removed relation from the PlaceHolderVars in the surviving rels' join clauses, including the sub-clauses of any OR clause. Like 9a60f295b, this is only reachable on v18 and later, where match_index_to_operand() began ignoring PlaceHolderVars. Author: Arne Roland Reviewed-by: Tender Wang Reviewed-by: Richard Guo Discussion: https://postgr.es/m/27a44087-3d65-473e-8d88-7c12228e0d7e@malkut.net Backpatch-through: 18 --- diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c index 5557600988d..4d19204da0a 100644 --- a/src/backend/optimizer/plan/analyzejoins.c +++ b/src/backend/optimizer/plan/analyzejoins.c @@ -75,6 +75,8 @@ static void remove_rel_from_restrictinfo(RestrictInfo *rinfo, static void remove_rel_from_eclass(PlannerInfo *root, EquivalenceClass *ec, SpecialJoinInfo *sjinfo, int relid, int subst); +static void remove_rel_from_restrictinfo_phvs(RestrictInfo *rinfo, + 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); @@ -347,6 +349,7 @@ remove_rel_from_query(PlannerInfo *root, RelOptInfo *rel, int relid = rel->relid; Index rti; ListCell *l; + Bitmapset *seen_serials = NULL; /* * Update all_baserels and related relid sets. @@ -522,9 +525,9 @@ remove_rel_from_query(PlannerInfo *root, RelOptInfo *rel, * lateral_vars lists. (We already did this above for ph_needed.) * * 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. + * PlaceHolderVar embedded in the surviving rels' restriction clauses and + * join clauses; 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++) { @@ -554,10 +557,27 @@ remove_rel_from_query(PlannerInfo *root, RelOptInfo *rel, if (sjinfo != NULL && rti != relid && root->glob->lastPHId != 0) { foreach_node(RestrictInfo, rinfo, otherrel->baserestrictinfo) + remove_rel_from_restrictinfo_phvs(rinfo, relid, sjinfo->ojrelid); + + /* + * Join clauses need the same treatment, but there's no value in + * processing any join clause more than once. So it's slightly + * annoying that we have to find them via the per-base-relation + * joininfo lists. Avoid duplicate processing by tracking the + * rinfo_serial numbers of join clauses we've already seen. (This + * doesn't work for is_clone clauses, so we must waste effort on + * them.) + */ + foreach_node(RestrictInfo, rinfo, otherrel->joininfo) { - rinfo->clause = (Expr *) - remove_rel_from_phvs((Node *) rinfo->clause, relid, - sjinfo->ojrelid); + if (!rinfo->is_clone) /* else serial number is not unique */ + { + if (bms_is_member(rinfo->rinfo_serial, seen_serials)) + continue; /* saw it already */ + seen_serials = bms_add_member(seen_serials, + rinfo->rinfo_serial); + } + remove_rel_from_restrictinfo_phvs(rinfo, relid, sjinfo->ojrelid); } } } @@ -847,6 +867,53 @@ remove_rel_from_eclass(PlannerInfo *root, EquivalenceClass *ec, ec_clear_derived_clauses(ec); } +/* + * Remove any references to relid or ojrelid from the PlaceHolderVars embedded + * in a RestrictInfo's clause. + * + * If it's an OR clause, we must also fix up the orclause, which is a parallel + * representation built from its own sub-RestrictInfos. We recurse into the + * sub-clauses for that, mirroring remove_rel_from_restrictinfo. + */ +static void +remove_rel_from_restrictinfo_phvs(RestrictInfo *rinfo, int relid, int ojrelid) +{ + rinfo->clause = (Expr *) + remove_rel_from_phvs((Node *) rinfo->clause, relid, ojrelid); + + /* If it's an OR, recurse to clean up sub-clauses */ + if (restriction_is_or_clause(rinfo)) + { + ListCell *lc; + + Assert(is_orclause(rinfo->orclause)); + foreach(lc, ((BoolExpr *) rinfo->orclause)->args) + { + Node *orarg = (Node *) lfirst(lc); + + /* OR arguments should be ANDs or sub-RestrictInfos */ + if (is_andclause(orarg)) + { + List *andargs = ((BoolExpr *) orarg)->args; + ListCell *lc2; + + foreach(lc2, andargs) + { + RestrictInfo *rinfo2 = lfirst_node(RestrictInfo, lc2); + + remove_rel_from_restrictinfo_phvs(rinfo2, relid, ojrelid); + } + } + else + { + RestrictInfo *rinfo2 = castNode(RestrictInfo, orarg); + + remove_rel_from_restrictinfo_phvs(rinfo2, relid, ojrelid); + } + } + } +} + /* * Remove any references to the specified RT index(es) from the phrels (and * phnullingrels) of every PlaceHolderVar in the given expression. diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index e9a5ecb1581..86078cbf27d 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -6394,6 +6394,17 @@ group by (); Result (1 row) +-- likewise for a PHV embedded in an OR join clause +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 = 1 and s.id = 2) or (t1.id = 3 and s.id = 4) +group by (); + QUERY PLAN +------------ + Result +(1 row) + 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 d0f36bec25c..3aaa882d668 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -2354,6 +2354,13 @@ select 1 from parted_b t1 on t1.id = s.id group by (); +-- likewise for a PHV embedded in an OR join clause +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 = 1 and s.id = 2) or (t1.id = 3 and s.id = 4) +group by (); + rollback; create temp table parent (k int primary key, pd int);