diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c index f109af25d72..881950e5264 100644 --- a/src/backend/optimizer/plan/analyzejoins.c +++ b/src/backend/optimizer/plan/analyzejoins.c @@ -64,6 +64,8 @@ static void remove_rel_from_restrictinfo(RestrictInfo *rinfo, int relid, int ojrelid); static void remove_rel_from_eclass(PlannerInfo *root, EquivalenceClass *ec, int relid, int ojrelid); +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); @@ -457,6 +459,7 @@ remove_rel_from_query(PlannerInfo *root, int relid, ListCell *l; bool is_outer_join = (sjinfo != NULL); bool is_self_join = (!is_outer_join && subst > 0); + Bitmapset *seen_serials = NULL; Assert(is_outer_join || is_self_join); Assert(!is_outer_join || ojrelid > 0); @@ -644,9 +647,9 @@ remove_rel_from_query(PlannerInfo *root, int relid, * 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. + * 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++) { @@ -676,9 +679,27 @@ remove_rel_from_query(PlannerInfo *root, int relid, if (is_outer_join && rti != relid && root->glob->lastPHId != 0) { foreach_node(RestrictInfo, rinfo, otherrel->baserestrictinfo) + remove_rel_from_restrictinfo_phvs(rinfo, relid, 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, 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, ojrelid); } } } @@ -844,6 +865,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 ed946abed7f..83bd5649d5c 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -6586,6 +6586,18 @@ group by (); Replaces: Aggregate (2 rows) +-- 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 + 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 78f7b4f544d..32d4a5a677e 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -2432,6 +2432,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);