diff options
| -rw-r--r-- | src/backend/optimizer/util/placeholder.c | 38 | ||||
| -rw-r--r-- | src/test/regress/expected/join.out | 64 | ||||
| -rw-r--r-- | src/test/regress/sql/join.sql | 27 |
3 files changed, 118 insertions, 11 deletions
diff --git a/src/backend/optimizer/util/placeholder.c b/src/backend/optimizer/util/placeholder.c index 6abdc0299b5..bf991018abb 100644 --- a/src/backend/optimizer/util/placeholder.c +++ b/src/backend/optimizer/util/placeholder.c @@ -404,8 +404,10 @@ add_placeholders_to_base_rels(PlannerInfo *root) * and if they contain lateral references, add those references to the * joinrel's direct_lateral_relids. * - * A join rel should emit a PlaceHolderVar if (a) the PHV is needed above - * this join level and (b) the PHV can be computed at or below this level. + * A join rel should emit a PlaceHolderVar if (a) the PHV can be computed + * at or below this join level and (b) the PHV is needed above this level. + * However, condition (a) is sufficient to add to direct_lateral_relids, + * as explained below. */ void add_placeholders_to_joinrel(PlannerInfo *root, RelOptInfo *joinrel, @@ -418,11 +420,11 @@ add_placeholders_to_joinrel(PlannerInfo *root, RelOptInfo *joinrel, { PlaceHolderInfo *phinfo = (PlaceHolderInfo *) lfirst(lc); - /* Is it still needed above this joinrel? */ - if (bms_nonempty_difference(phinfo->ph_needed, relids)) + /* Is it computable here? */ + if (bms_is_subset(phinfo->ph_eval_at, relids)) { - /* Is it computable here? */ - if (bms_is_subset(phinfo->ph_eval_at, relids)) + /* Is it still needed above this joinrel? */ + if (bms_nonempty_difference(phinfo->ph_needed, relids)) { /* Yup, add it to the output */ joinrel->reltarget->exprs = lappend(joinrel->reltarget->exprs, @@ -450,12 +452,26 @@ add_placeholders_to_joinrel(PlannerInfo *root, RelOptInfo *joinrel, joinrel->reltarget->cost.startup += cost.startup; joinrel->reltarget->cost.per_tuple += cost.per_tuple; } - - /* Adjust joinrel's direct_lateral_relids as needed */ - joinrel->direct_lateral_relids = - bms_add_members(joinrel->direct_lateral_relids, - phinfo->ph_lateral); } + + /* + * Also adjust joinrel's direct_lateral_relids to include the + * PHV's source rel(s). We must do this even if we're not + * actually going to emit the PHV, otherwise join_is_legal() will + * reject valid join orderings. (In principle maybe we could + * instead remove the joinrel's lateral_relids dependency; but + * that's complicated to get right, and cases where we're not + * going to emit the PHV are too rare to justify the work.) + * + * In principle we should only do this if the join doesn't yet + * include the PHV's source rel(s). But our caller + * build_join_rel() will clean things up by removing the join's + * own relids from its direct_lateral_relids, so we needn't + * account for that here. + */ + joinrel->direct_lateral_relids = + bms_add_members(joinrel->direct_lateral_relids, + phinfo->ph_lateral); } } } diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index 60b621b651f..a1180417319 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -3011,6 +3011,70 @@ order by 1,2; (8 rows) -- +-- variant where a PlaceHolderVar is needed at a join, but not above the join +-- +explain (costs off) +select * from + int4_tbl as i41, + lateral + (select 1 as x from + (select i41.f1 as lat, + i42.f1 as loc from + int8_tbl as i81, int4_tbl as i42) as ss1 + right join int4_tbl as i43 on (i43.f1 > 1) + where ss1.loc = ss1.lat) as ss2 +where i41.f1 > 0; + QUERY PLAN +-------------------------------------------------- + Nested Loop + -> Nested Loop + -> Seq Scan on int4_tbl i41 + Filter: (f1 > 0) + -> Nested Loop + Join Filter: (i41.f1 = i42.f1) + -> Seq Scan on int8_tbl i81 + -> Materialize + -> Seq Scan on int4_tbl i42 + -> Materialize + -> Seq Scan on int4_tbl i43 + Filter: (f1 > 1) +(12 rows) + +select * from + int4_tbl as i41, + lateral + (select 1 as x from + (select i41.f1 as lat, + i42.f1 as loc from + int8_tbl as i81, int4_tbl as i42) as ss1 + right join int4_tbl as i43 on (i43.f1 > 1) + where ss1.loc = ss1.lat) as ss2 +where i41.f1 > 0; + f1 | x +------------+--- + 123456 | 1 + 123456 | 1 + 123456 | 1 + 123456 | 1 + 123456 | 1 + 123456 | 1 + 123456 | 1 + 123456 | 1 + 123456 | 1 + 123456 | 1 + 2147483647 | 1 + 2147483647 | 1 + 2147483647 | 1 + 2147483647 | 1 + 2147483647 | 1 + 2147483647 | 1 + 2147483647 | 1 + 2147483647 | 1 + 2147483647 | 1 + 2147483647 | 1 +(20 rows) + +-- -- test the corner cases FULL JOIN ON TRUE and FULL JOIN ON FALSE -- select * from int4_tbl a full join int4_tbl b on true; diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql index d687216618c..4de24c19040 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -906,6 +906,33 @@ where order by 1,2; -- +-- variant where a PlaceHolderVar is needed at a join, but not above the join +-- + +explain (costs off) +select * from + int4_tbl as i41, + lateral + (select 1 as x from + (select i41.f1 as lat, + i42.f1 as loc from + int8_tbl as i81, int4_tbl as i42) as ss1 + right join int4_tbl as i43 on (i43.f1 > 1) + where ss1.loc = ss1.lat) as ss2 +where i41.f1 > 0; + +select * from + int4_tbl as i41, + lateral + (select 1 as x from + (select i41.f1 as lat, + i42.f1 as loc from + int8_tbl as i81, int4_tbl as i42) as ss1 + right join int4_tbl as i43 on (i43.f1 > 1) + where ss1.loc = ss1.lat) as ss2 +where i41.f1 > 0; + +-- -- test the corner cases FULL JOIN ON TRUE and FULL JOIN ON FALSE -- select * from int4_tbl a full join int4_tbl b on true; |