Skip to content

Commit 8c5da2d

Browse files
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 e2d5de1 commit 8c5da2d

File tree

3 files changed

+118
-12
lines changed

3 files changed

+118
-12
lines changed

src/backend/optimizer/util/placeholder.c

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -400,10 +400,10 @@ add_placeholders_to_base_rels(PlannerInfo *root)
400400
* and if they contain lateral references, add those references to the
401401
* joinrel's direct_lateral_relids.
402402
*
403-
* A join rel should emit a PlaceHolderVar if (a) the PHV is needed above
404-
* this join level and (b) the PHV can be computed at or below this level.
405-
* At this time we do not need to distinguish whether the PHV will be
406-
* computed here or copied up from below.
403+
* A join rel should emit a PlaceHolderVar if (a) the PHV can be computed
404+
* at or below this join level and (b) the PHV is needed above this level.
405+
* However, condition (a) is sufficient to add to direct_lateral_relids,
406+
* as explained below.
407407
*/
408408
void
409409
add_placeholders_to_joinrel(PlannerInfo *root, RelOptInfo *joinrel)
@@ -415,21 +415,36 @@ add_placeholders_to_joinrel(PlannerInfo *root, RelOptInfo *joinrel)
415415
{
416416
PlaceHolderInfo *phinfo = (PlaceHolderInfo *) lfirst(lc);
417417

418-
/* Is it still needed above this joinrel? */
419-
if (bms_nonempty_difference(phinfo->ph_needed, relids))
418+
/* Is it computable here? */
419+
if (bms_is_subset(phinfo->ph_eval_at, relids))
420420
{
421-
/* Is it computable here? */
422-
if (bms_is_subset(phinfo->ph_eval_at, relids))
421+
/* Is it still needed above this joinrel? */
422+
if (bms_nonempty_difference(phinfo->ph_needed, relids))
423423
{
424424
/* Yup, add it to the output */
425425
joinrel->reltargetlist = lappend(joinrel->reltargetlist,
426426
phinfo->ph_var);
427427
joinrel->width += phinfo->ph_width;
428-
/* Adjust joinrel's direct_lateral_relids as needed */
429-
joinrel->direct_lateral_relids =
430-
bms_add_members(joinrel->direct_lateral_relids,
431-
phinfo->ph_lateral);
432428
}
429+
430+
/*
431+
* Also adjust joinrel's direct_lateral_relids to include the
432+
* PHV's source rel(s). We must do this even if we're not
433+
* actually going to emit the PHV, otherwise join_is_legal() will
434+
* reject valid join orderings. (In principle maybe we could
435+
* instead remove the joinrel's lateral_relids dependency; but
436+
* that's complicated to get right, and cases where we're not
437+
* going to emit the PHV are too rare to justify the work.)
438+
*
439+
* In principle we should only do this if the join doesn't yet
440+
* include the PHV's source rel(s). But our caller
441+
* build_join_rel() will clean things up by removing the join's
442+
* own relids from its direct_lateral_relids, so we needn't
443+
* account for that here.
444+
*/
445+
joinrel->direct_lateral_relids =
446+
bms_add_members(joinrel->direct_lateral_relids,
447+
phinfo->ph_lateral);
433448
}
434449
}
435450
}

src/test/regress/expected/join.out

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2972,6 +2972,70 @@ order by 1,2;
29722972
4567890123456789 | 4567890123456789 | 4567890123456789 | 42
29732973
(8 rows)
29742974

2975+
--
2976+
-- variant where a PlaceHolderVar is needed at a join, but not above the join
2977+
--
2978+
explain (costs off)
2979+
select * from
2980+
int4_tbl as i41,
2981+
lateral
2982+
(select 1 as x from
2983+
(select i41.f1 as lat,
2984+
i42.f1 as loc from
2985+
int8_tbl as i81, int4_tbl as i42) as ss1
2986+
right join int4_tbl as i43 on (i43.f1 > 1)
2987+
where ss1.loc = ss1.lat) as ss2
2988+
where i41.f1 > 0;
2989+
QUERY PLAN
2990+
--------------------------------------------------
2991+
Nested Loop
2992+
-> Nested Loop
2993+
-> Seq Scan on int4_tbl i41
2994+
Filter: (f1 > 0)
2995+
-> Nested Loop
2996+
Join Filter: (i41.f1 = i42.f1)
2997+
-> Seq Scan on int8_tbl i81
2998+
-> Materialize
2999+
-> Seq Scan on int4_tbl i42
3000+
-> Materialize
3001+
-> Seq Scan on int4_tbl i43
3002+
Filter: (f1 > 1)
3003+
(12 rows)
3004+
3005+
select * from
3006+
int4_tbl as i41,
3007+
lateral
3008+
(select 1 as x from
3009+
(select i41.f1 as lat,
3010+
i42.f1 as loc from
3011+
int8_tbl as i81, int4_tbl as i42) as ss1
3012+
right join int4_tbl as i43 on (i43.f1 > 1)
3013+
where ss1.loc = ss1.lat) as ss2
3014+
where i41.f1 > 0;
3015+
f1 | x
3016+
------------+---
3017+
123456 | 1
3018+
123456 | 1
3019+
123456 | 1
3020+
123456 | 1
3021+
123456 | 1
3022+
123456 | 1
3023+
123456 | 1
3024+
123456 | 1
3025+
123456 | 1
3026+
123456 | 1
3027+
2147483647 | 1
3028+
2147483647 | 1
3029+
2147483647 | 1
3030+
2147483647 | 1
3031+
2147483647 | 1
3032+
2147483647 | 1
3033+
2147483647 | 1
3034+
2147483647 | 1
3035+
2147483647 | 1
3036+
2147483647 | 1
3037+
(20 rows)
3038+
29753039
--
29763040
-- test the corner cases FULL JOIN ON TRUE and FULL JOIN ON FALSE
29773041
--

src/test/regress/sql/join.sql

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

885+
--
886+
-- variant where a PlaceHolderVar is needed at a join, but not above the join
887+
--
888+
889+
explain (costs off)
890+
select * from
891+
int4_tbl as i41,
892+
lateral
893+
(select 1 as x from
894+
(select i41.f1 as lat,
895+
i42.f1 as loc from
896+
int8_tbl as i81, int4_tbl as i42) as ss1
897+
right join int4_tbl as i43 on (i43.f1 > 1)
898+
where ss1.loc = ss1.lat) as ss2
899+
where i41.f1 > 0;
900+
901+
select * from
902+
int4_tbl as i41,
903+
lateral
904+
(select 1 as x from
905+
(select i41.f1 as lat,
906+
i42.f1 as loc from
907+
int8_tbl as i81, int4_tbl as i42) as ss1
908+
right join int4_tbl as i43 on (i43.f1 > 1)
909+
where ss1.loc = ss1.lat) as ss2
910+
where i41.f1 > 0;
911+
885912
--
886913
-- test the corner cases FULL JOIN ON TRUE and FULL JOIN ON FALSE
887914
--

0 commit comments

Comments
 (0)