From 8c5da2d512e3e00c55f3f8fb7af12dfabc5b9284 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 30 Nov 2020 12:22:43 -0500 Subject: [PATCH] Fix miscomputation of direct_lateral_relids for join relations. If a PlaceHolderVar is to be evaluated at a join relation, but its value is only needed there and not at higher levels, we neglected to update the joinrel's direct_lateral_relids to include the PHV's source rel. This causes problems because join_is_legal() then won't allow joining the joinrel to the PHV's source rel at all, leading to "failed to build any N-way joins" planner failures. Per report from Andreas Seltenreich. Back-patch to 9.5 where the problem originated. Discussion: https://postgr.es/m/87blfgqa4t.fsf@aurora.ydns.eu --- src/backend/optimizer/util/placeholder.c | 39 ++++++++++----- src/test/regress/expected/join.out | 64 ++++++++++++++++++++++++ src/test/regress/sql/join.sql | 27 ++++++++++ 3 files changed, 118 insertions(+), 12 deletions(-) diff --git a/src/backend/optimizer/util/placeholder.c b/src/backend/optimizer/util/placeholder.c index 7fa93fb29a..aee8d2b475 100644 --- a/src/backend/optimizer/util/placeholder.c +++ b/src/backend/optimizer/util/placeholder.c @@ -400,10 +400,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. - * At this time we do not need to distinguish whether the PHV will be - * computed here or copied up from below. + * 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) @@ -415,21 +415,36 @@ 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->reltargetlist = lappend(joinrel->reltargetlist, phinfo->ph_var); joinrel->width += phinfo->ph_width; - /* 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 7ee0d122cb..c901e4f0f2 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -2972,6 +2972,70 @@ order by 1,2; 4567890123456789 | 4567890123456789 | 4567890123456789 | 42 (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 -- diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql index b0b83893dc..3199ad5b65 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -882,6 +882,33 @@ where 1 = (select 1 from int8_tbl t3 where ss.y is not null limit 1) 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 --