Skip to content

Commit b1738ff

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 873ea9e commit b1738ff

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
@@ -3010,6 +3010,70 @@ order by 1,2;
30103010
4567890123456789 | 4567890123456789 | 4567890123456789 | 42
30113011
(8 rows)
30123012

3013+
--
3014+
-- variant where a PlaceHolderVar is needed at a join, but not above the join
3015+
--
3016+
explain (costs off)
3017+
select * from
3018+
int4_tbl as i41,
3019+
lateral
3020+
(select 1 as x from
3021+
(select i41.f1 as lat,
3022+
i42.f1 as loc from
3023+
int8_tbl as i81, int4_tbl as i42) as ss1
3024+
right join int4_tbl as i43 on (i43.f1 > 1)
3025+
where ss1.loc = ss1.lat) as ss2
3026+
where i41.f1 > 0;
3027+
QUERY PLAN
3028+
--------------------------------------------------
3029+
Nested Loop
3030+
-> Nested Loop
3031+
-> Seq Scan on int4_tbl i41
3032+
Filter: (f1 > 0)
3033+
-> Nested Loop
3034+
Join Filter: (i41.f1 = i42.f1)
3035+
-> Seq Scan on int8_tbl i81
3036+
-> Materialize
3037+
-> Seq Scan on int4_tbl i42
3038+
-> Materialize
3039+
-> Seq Scan on int4_tbl i43
3040+
Filter: (f1 > 1)
3041+
(12 rows)
3042+
3043+
select * from
3044+
int4_tbl as i41,
3045+
lateral
3046+
(select 1 as x from
3047+
(select i41.f1 as lat,
3048+
i42.f1 as loc from
3049+
int8_tbl as i81, int4_tbl as i42) as ss1
3050+
right join int4_tbl as i43 on (i43.f1 > 1)
3051+
where ss1.loc = ss1.lat) as ss2
3052+
where i41.f1 > 0;
3053+
f1 | x
3054+
------------+---
3055+
123456 | 1
3056+
123456 | 1
3057+
123456 | 1
3058+
123456 | 1
3059+
123456 | 1
3060+
123456 | 1
3061+
123456 | 1
3062+
123456 | 1
3063+
123456 | 1
3064+
123456 | 1
3065+
2147483647 | 1
3066+
2147483647 | 1
3067+
2147483647 | 1
3068+
2147483647 | 1
3069+
2147483647 | 1
3070+
2147483647 | 1
3071+
2147483647 | 1
3072+
2147483647 | 1
3073+
2147483647 | 1
3074+
2147483647 | 1
3075+
(20 rows)
3076+
30133077
--
30143078
-- test the corner cases FULL JOIN ON TRUE and FULL JOIN ON FALSE
30153079
--

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

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

908+
--
909+
-- variant where a PlaceHolderVar is needed at a join, but not above the join
910+
--
911+
912+
explain (costs off)
913+
select * from
914+
int4_tbl as i41,
915+
lateral
916+
(select 1 as x from
917+
(select i41.f1 as lat,
918+
i42.f1 as loc from
919+
int8_tbl as i81, int4_tbl as i42) as ss1
920+
right join int4_tbl as i43 on (i43.f1 > 1)
921+
where ss1.loc = ss1.lat) as ss2
922+
where i41.f1 > 0;
923+
924+
select * from
925+
int4_tbl as i41,
926+
lateral
927+
(select 1 as x from
928+
(select i41.f1 as lat,
929+
i42.f1 as loc from
930+
int8_tbl as i81, int4_tbl as i42) as ss1
931+
right join int4_tbl as i43 on (i43.f1 > 1)
932+
where ss1.loc = ss1.lat) as ss2
933+
where i41.f1 > 0;
934+
908935
--
909936
-- test the corner cases FULL JOIN ON TRUE and FULL JOIN ON FALSE
910937
--

0 commit comments

Comments
 (0)