Skip to content

Commit 7950657

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 ed089d2 commit 7950657

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
@@ -470,11 +470,30 @@ join_is_legal(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,
470470
{
471471
/*
472472
* Otherwise, the proposed join overlaps the RHS but isn't a valid
473-
* implementation of this SJ. It might still be a legal join,
474-
* however, if we're allowed to associate it into the RHS of this
475-
* SJ. That means this SJ must be a LEFT join (not SEMI or ANTI,
476-
* and certainly not FULL) and the proposed join must not overlap
477-
* the LHS.
473+
* implementation of this SJ. But don't panic quite yet: the RHS
474+
* violation might have occurred previously, in one or both input
475+
* relations, in which case we must have previously decided that
476+
* it was OK to commute some other SJ with this one. If we need
477+
* to perform this join to finish building up the RHS, rejecting
478+
* it could lead to not finding any plan at all. (This can occur
479+
* because of the heuristics elsewhere in this file that postpone
480+
* clauseless joins: we might not consider doing a clauseless join
481+
* within the RHS until after we've performed other, validly
482+
* commutable SJs with one or both sides of the clauseless join.)
483+
* This consideration boils down to the rule that if both inputs
484+
* overlap the RHS, we can allow the join --- they are either
485+
* fully within the RHS, or represent previously-allowed joins to
486+
* rels outside it.
487+
*/
488+
if (bms_overlap(rel1->relids, sjinfo->min_righthand) &&
489+
bms_overlap(rel2->relids, sjinfo->min_righthand))
490+
continue; /* assume valid previous violation of RHS */
491+
492+
/*
493+
* The proposed join could still be legal, but only if we're
494+
* allowed to associate it into the RHS of this SJ. That means
495+
* this SJ must be a LEFT join (not SEMI or ANTI, and certainly
496+
* not FULL) and the proposed join must not overlap the LHS.
478497
*/
479498
if (sjinfo->jointype != JOIN_LEFT ||
480499
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
@@ -3449,6 +3449,52 @@ select t1.* from
34493449
hi de ho neighbor
34503450
(2 rows)
34513451

3452+
explain (verbose, costs off)
3453+
select * from
3454+
text_tbl t1
3455+
inner join int8_tbl i8
3456+
on i8.q2 = 456
3457+
right join text_tbl t2
3458+
on t1.f1 = 'doh!'
3459+
left join int4_tbl i4
3460+
on i8.q1 = i4.f1;
3461+
QUERY PLAN
3462+
--------------------------------------------------------
3463+
Nested Loop Left Join
3464+
Output: t1.f1, i8.q1, i8.q2, t2.f1, i4.f1
3465+
-> Seq Scan on public.text_tbl t2
3466+
Output: t2.f1
3467+
-> Materialize
3468+
Output: i8.q1, i8.q2, i4.f1, t1.f1
3469+
-> Nested Loop
3470+
Output: i8.q1, i8.q2, i4.f1, t1.f1
3471+
-> Nested Loop Left Join
3472+
Output: i8.q1, i8.q2, i4.f1
3473+
Join Filter: (i8.q1 = i4.f1)
3474+
-> Seq Scan on public.int8_tbl i8
3475+
Output: i8.q1, i8.q2
3476+
Filter: (i8.q2 = 456)
3477+
-> Seq Scan on public.int4_tbl i4
3478+
Output: i4.f1
3479+
-> Seq Scan on public.text_tbl t1
3480+
Output: t1.f1
3481+
Filter: (t1.f1 = 'doh!'::text)
3482+
(19 rows)
3483+
3484+
select * from
3485+
text_tbl t1
3486+
inner join int8_tbl i8
3487+
on i8.q2 = 456
3488+
right join text_tbl t2
3489+
on t1.f1 = 'doh!'
3490+
left join int4_tbl i4
3491+
on i8.q1 = i4.f1;
3492+
f1 | q1 | q2 | f1 | f1
3493+
------+-----+-----+-------------------+----
3494+
doh! | 123 | 456 | doh! |
3495+
doh! | 123 | 456 | hi de ho neighbor |
3496+
(2 rows)
3497+
34523498
--
34533499
-- test ability to push constants through outer join clauses
34543500
--

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

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

1079+
explain (verbose, costs off)
1080+
select * from
1081+
text_tbl t1
1082+
inner join int8_tbl i8
1083+
on i8.q2 = 456
1084+
right join text_tbl t2
1085+
on t1.f1 = 'doh!'
1086+
left join int4_tbl i4
1087+
on i8.q1 = i4.f1;
1088+
1089+
select * from
1090+
text_tbl t1
1091+
inner join int8_tbl i8
1092+
on i8.q2 = 456
1093+
right join text_tbl t2
1094+
on t1.f1 = 'doh!'
1095+
left join int4_tbl i4
1096+
on i8.q1 = i4.f1;
1097+
10791098
--
10801099
-- test ability to push constants through outer join clauses
10811100
--

0 commit comments

Comments
 (0)