Skip to content

Commit caecab2

Browse files
tglsfdctglsfdc
committed
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
1 parent 777ac03 commit caecab2

File tree

3 files changed

+118
-11
lines changed

3 files changed

+118
-11
lines changed

‎src/backend/optimizer/util/placeholder.c

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -404,8 +404,10 @@ add_placeholders_to_base_rels(PlannerInfo *root)
404404
* and if they contain lateral references, add those references to the
405405
* joinrel's direct_lateral_relids.
406406
*
407-
* A join rel should emit a PlaceHolderVar if (a) the PHV is needed above
408-
* this join level and (b) the PHV can be computed at or below this level.
407+
* A join rel should emit a PlaceHolderVar if (a) the PHV can be computed
408+
* at or below this join level and (b) the PHV is needed above this level.
409+
* However, condition (a) is sufficient to add to direct_lateral_relids,
410+
* as explained below.
409411
*/
410412
void
411413
add_placeholders_to_joinrel(PlannerInfo *root, RelOptInfo *joinrel,
@@ -418,11 +420,11 @@ add_placeholders_to_joinrel(PlannerInfo *root, RelOptInfo *joinrel,
418420
{
419421
PlaceHolderInfo *phinfo = (PlaceHolderInfo *) lfirst(lc);
420422

421-
/* Is it still needed above this joinrel? */
422-
if (bms_nonempty_difference(phinfo->ph_needed, relids))
423+
/* Is it computable here? */
424+
if (bms_is_subset(phinfo->ph_eval_at, relids))
423425
{
424-
/* Is it computable here? */
425-
if (bms_is_subset(phinfo->ph_eval_at, relids))
426+
/* Is it still needed above this joinrel? */
427+
if (bms_nonempty_difference(phinfo->ph_needed, relids))
426428
{
427429
/* Yup, add it to the output */
428430
joinrel->reltarget->exprs = lappend(joinrel->reltarget->exprs,
@@ -450,12 +452,26 @@ add_placeholders_to_joinrel(PlannerInfo *root, RelOptInfo *joinrel,
450452
joinrel->reltarget->cost.startup += cost.startup;
451453
joinrel->reltarget->cost.per_tuple += cost.per_tuple;
452454
}
453-
454-
/* Adjust joinrel's direct_lateral_relids as needed */
455-
joinrel->direct_lateral_relids =
456-
bms_add_members(joinrel->direct_lateral_relids,
457-
phinfo->ph_lateral);
458455
}
456+
457+
/*
458+
* Also adjust joinrel's direct_lateral_relids to include the
459+
* PHV's source rel(s). We must do this even if we're not
460+
* actually going to emit the PHV, otherwise join_is_legal() will
461+
* reject valid join orderings. (In principle maybe we could
462+
* instead remove the joinrel's lateral_relids dependency; but
463+
* that's complicated to get right, and cases where we're not
464+
* going to emit the PHV are too rare to justify the work.)
465+
*
466+
* In principle we should only do this if the join doesn't yet
467+
* include the PHV's source rel(s). But our caller
468+
* build_join_rel() will clean things up by removing the join's
469+
* own relids from its direct_lateral_relids, so we needn't
470+
* account for that here.
471+
*/
472+
joinrel->direct_lateral_relids =
473+
bms_add_members(joinrel->direct_lateral_relids,
474+
phinfo->ph_lateral);
459475
}
460476
}
461477
}

‎src/test/regress/expected/join.out

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2994,6 +2994,70 @@ order by 1,2;
29942994
4567890123456789 | 4567890123456789 | 4567890123456789 | 42
29952995
(8 rows)
29962996

2997+
--
2998+
-- variant where a PlaceHolderVar is needed at a join, but not above the join
2999+
--
3000+
explain (costs off)
3001+
select * from
3002+
int4_tbl as i41,
3003+
lateral
3004+
(select 1 as x from
3005+
(select i41.f1 as lat,
3006+
i42.f1 as loc from
3007+
int8_tbl as i81, int4_tbl as i42) as ss1
3008+
right join int4_tbl as i43 on (i43.f1 > 1)
3009+
where ss1.loc = ss1.lat) as ss2
3010+
where i41.f1 > 0;
3011+
QUERY PLAN
3012+
--------------------------------------------------
3013+
Nested Loop
3014+
-> Nested Loop
3015+
-> Seq Scan on int4_tbl i41
3016+
Filter: (f1 > 0)
3017+
-> Nested Loop
3018+
Join Filter: (i41.f1 = i42.f1)
3019+
-> Seq Scan on int8_tbl i81
3020+
-> Materialize
3021+
-> Seq Scan on int4_tbl i42
3022+
-> Materialize
3023+
-> Seq Scan on int4_tbl i43
3024+
Filter: (f1 > 1)
3025+
(12 rows)
3026+
3027+
select * from
3028+
int4_tbl as i41,
3029+
lateral
3030+
(select 1 as x from
3031+
(select i41.f1 as lat,
3032+
i42.f1 as loc from
3033+
int8_tbl as i81, int4_tbl as i42) as ss1
3034+
right join int4_tbl as i43 on (i43.f1 > 1)
3035+
where ss1.loc = ss1.lat) as ss2
3036+
where i41.f1 > 0;
3037+
f1 | x
3038+
------------+---
3039+
123456 | 1
3040+
123456 | 1
3041+
123456 | 1
3042+
123456 | 1
3043+
123456 | 1
3044+
123456 | 1
3045+
123456 | 1
3046+
123456 | 1
3047+
123456 | 1
3048+
123456 | 1
3049+
2147483647 | 1
3050+
2147483647 | 1
3051+
2147483647 | 1
3052+
2147483647 | 1
3053+
2147483647 | 1
3054+
2147483647 | 1
3055+
2147483647 | 1
3056+
2147483647 | 1
3057+
2147483647 | 1
3058+
2147483647 | 1
3059+
(20 rows)
3060+
29973061
--
29983062
-- test the corner cases FULL JOIN ON TRUE and FULL JOIN ON FALSE
29993063
--

‎src/test/regress/sql/join.sql

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -891,6 +891,33 @@ where
891891
1 = (select 1 from int8_tbl t3 where ss.y is not null limit 1)
892892
order by 1,2;
893893

894+
--
895+
-- variant where a PlaceHolderVar is needed at a join, but not above the join
896+
--
897+
898+
explain (costs off)
899+
select * from
900+
int4_tbl as i41,
901+
lateral
902+
(select 1 as x from
903+
(select i41.f1 as lat,
904+
i42.f1 as loc from
905+
int8_tbl as i81, int4_tbl as i42) as ss1
906+
right join int4_tbl as i43 on (i43.f1 > 1)
907+
where ss1.loc = ss1.lat) as ss2
908+
where i41.f1 > 0;
909+
910+
select * from
911+
int4_tbl as i41,
912+
lateral
913+
(select 1 as x from
914+
(select i41.f1 as lat,
915+
i42.f1 as loc from
916+
int8_tbl as i81, int4_tbl as i42) as ss1
917+
right join int4_tbl as i43 on (i43.f1 > 1)
918+
where ss1.loc = ss1.lat) as ss2
919+
where i41.f1 > 0;
920+
894921
--
895922
-- test the corner cases FULL JOIN ON TRUE and FULL JOIN ON FALSE
896923
--

0 commit comments

Comments
 (0)