Skip to content

Commit 7b4b57f

Browse files
tglsfdctglsfdc
committed
Undo mistaken tightening in join_is_legal().
One of the changes I made in commit 8703059 turns out not to have been such a good idea: we still need the exception in join_is_legal() that allows a join if both inputs already overlap the RHS of the special join we're checking. Otherwise we can miss valid plans, and might indeed fail to find a plan at all, as in recent report from Andreas Seltenreich. That code was added way back in commit c171176, but I failed to include a regression test case then; my bad. Put it back with a better explanation, and a test this time. The logic does end up a bit different than before though: I now believe it's appropriate to make this check first, thereby allowing such a case whether or not we'd consider the previous SJ(s) to commute with this one. (Presumably, we already decided they did; but it was confusing to have this consideration in the middle of the code that was handling the other case.) Back-patch to all active branches, like the previous patch.
1 parent d421595 commit 7b4b57f

File tree

3 files changed

+89
-5
lines changed

3 files changed

+89
-5
lines changed

‎src/backend/optimizer/path/joinrels.c

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -462,11 +462,30 @@ join_is_legal(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,
462462
{
463463
/*
464464
* Otherwise, the proposed join overlaps the RHS but isn't a valid
465-
* implementation of this SJ. It might still be a legal join,
466-
* however, if we're allowed to associate it into the RHS of this
467-
* SJ. That means this SJ must be a LEFT join (not SEMI or ANTI,
468-
* and certainly not FULL) and the proposed join must not overlap
469-
* the LHS.
465+
* implementation of this SJ. But don't panic quite yet: the RHS
466+
* violation might have occurred previously, in one or both input
467+
* relations, in which case we must have previously decided that
468+
* it was OK to commute some other SJ with this one. If we need
469+
* to perform this join to finish building up the RHS, rejecting
470+
* it could lead to not finding any plan at all. (This can occur
471+
* because of the heuristics elsewhere in this file that postpone
472+
* clauseless joins: we might not consider doing a clauseless join
473+
* within the RHS until after we've performed other, validly
474+
* commutable SJs with one or both sides of the clauseless join.)
475+
* This consideration boils down to the rule that if both inputs
476+
* overlap the RHS, we can allow the join --- they are either
477+
* fully within the RHS, or represent previously-allowed joins to
478+
* rels outside it.
479+
*/
480+
if (bms_overlap(rel1->relids, sjinfo->min_righthand) &&
481+
bms_overlap(rel2->relids, sjinfo->min_righthand))
482+
continue; /* assume valid previous violation of RHS */
483+
484+
/*
485+
* The proposed join could still be legal, but only if we're
486+
* allowed to associate it into the RHS of this SJ. That means
487+
* this SJ must be a LEFT join (not SEMI or ANTI, and certainly
488+
* not FULL) and the proposed join must not overlap the LHS.
470489
*/
471490
if (sjinfo->jointype != JOIN_LEFT ||
472491
bms_overlap(joinrelids, sjinfo->min_lefthand))

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

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2965,6 +2965,52 @@ select t1.* from
29652965
hi de ho neighbor
29662966
(2 rows)
29672967

2968+
explain (verbose, costs off)
2969+
select * from
2970+
text_tbl t1
2971+
inner join int8_tbl i8
2972+
on i8.q2 = 456
2973+
right join text_tbl t2
2974+
on t1.f1 = 'doh!'
2975+
left join int4_tbl i4
2976+
on i8.q1 = i4.f1;
2977+
QUERY PLAN
2978+
--------------------------------------------------------
2979+
Nested Loop Left Join
2980+
Output: t1.f1, i8.q1, i8.q2, t2.f1, i4.f1
2981+
-> Seq Scan on public.text_tbl t2
2982+
Output: t2.f1
2983+
-> Materialize
2984+
Output: i8.q1, i8.q2, i4.f1, t1.f1
2985+
-> Nested Loop
2986+
Output: i8.q1, i8.q2, i4.f1, t1.f1
2987+
-> Nested Loop Left Join
2988+
Output: i8.q1, i8.q2, i4.f1
2989+
Join Filter: (i8.q1 = i4.f1)
2990+
-> Seq Scan on public.int8_tbl i8
2991+
Output: i8.q1, i8.q2
2992+
Filter: (i8.q2 = 456)
2993+
-> Seq Scan on public.int4_tbl i4
2994+
Output: i4.f1
2995+
-> Seq Scan on public.text_tbl t1
2996+
Output: t1.f1
2997+
Filter: (t1.f1 = 'doh!'::text)
2998+
(19 rows)
2999+
3000+
select * from
3001+
text_tbl t1
3002+
inner join int8_tbl i8
3003+
on i8.q2 = 456
3004+
right join text_tbl t2
3005+
on t1.f1 = 'doh!'
3006+
left join int4_tbl i4
3007+
on i8.q1 = i4.f1;
3008+
f1 | q1 | q2 | f1 | f1
3009+
------+-----+-----+-------------------+----
3010+
doh! | 123 | 456 | doh! |
3011+
doh! | 123 | 456 | hi de ho neighbor |
3012+
(2 rows)
3013+
29683014
--
29693015
-- test ability to push constants through outer join clauses
29703016
--

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -811,6 +811,25 @@ select t1.* from
811811
left join int4_tbl i4
812812
on (i8.q2 = i4.f1);
813813

814+
explain (verbose, costs off)
815+
select * from
816+
text_tbl t1
817+
inner join int8_tbl i8
818+
on i8.q2 = 456
819+
right join text_tbl t2
820+
on t1.f1 = 'doh!'
821+
left join int4_tbl i4
822+
on i8.q1 = i4.f1;
823+
824+
select * from
825+
text_tbl t1
826+
inner join int8_tbl i8
827+
on i8.q2 = 456
828+
right join text_tbl t2
829+
on t1.f1 = 'doh!'
830+
left join int4_tbl i4
831+
on i8.q1 = i4.f1;
832+
814833
--
815834
-- test ability to push constants through outer join clauses
816835
--

0 commit comments

Comments
 (0)