🌐 AI搜索 & 代理 主页
Skip to content

Commit ec94bc1

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 fc0a640 commit ec94bc1

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
@@ -3563,6 +3563,52 @@ select t1.* from
35633563
hi de ho neighbor
35643564
(2 rows)
35653565

3566+
explain (verbose, costs off)
3567+
select * from
3568+
text_tbl t1
3569+
inner join int8_tbl i8
3570+
on i8.q2 = 456
3571+
right join text_tbl t2
3572+
on t1.f1 = 'doh!'
3573+
left join int4_tbl i4
3574+
on i8.q1 = i4.f1;
3575+
QUERY PLAN
3576+
--------------------------------------------------------
3577+
Nested Loop Left Join
3578+
Output: t1.f1, i8.q1, i8.q2, t2.f1, i4.f1
3579+
-> Seq Scan on public.text_tbl t2
3580+
Output: t2.f1
3581+
-> Materialize
3582+
Output: i8.q1, i8.q2, i4.f1, t1.f1
3583+
-> Nested Loop
3584+
Output: i8.q1, i8.q2, i4.f1, t1.f1
3585+
-> Nested Loop Left Join
3586+
Output: i8.q1, i8.q2, i4.f1
3587+
Join Filter: (i8.q1 = i4.f1)
3588+
-> Seq Scan on public.int8_tbl i8
3589+
Output: i8.q1, i8.q2
3590+
Filter: (i8.q2 = 456)
3591+
-> Seq Scan on public.int4_tbl i4
3592+
Output: i4.f1
3593+
-> Seq Scan on public.text_tbl t1
3594+
Output: t1.f1
3595+
Filter: (t1.f1 = 'doh!'::text)
3596+
(19 rows)
3597+
3598+
select * from
3599+
text_tbl t1
3600+
inner join int8_tbl i8
3601+
on i8.q2 = 456
3602+
right join text_tbl t2
3603+
on t1.f1 = 'doh!'
3604+
left join int4_tbl i4
3605+
on i8.q1 = i4.f1;
3606+
f1 | q1 | q2 | f1 | f1
3607+
------+-----+-----+-------------------+----
3608+
doh! | 123 | 456 | doh! |
3609+
doh! | 123 | 456 | hi de ho neighbor |
3610+
(2 rows)
3611+
35663612
--
35673613
-- test ability to push constants through outer join clauses
35683614
--

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

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

1111+
explain (verbose, costs off)
1112+
select * from
1113+
text_tbl t1
1114+
inner join int8_tbl i8
1115+
on i8.q2 = 456
1116+
right join text_tbl t2
1117+
on t1.f1 = 'doh!'
1118+
left join int4_tbl i4
1119+
on i8.q1 = i4.f1;
1120+
1121+
select * from
1122+
text_tbl t1
1123+
inner join int8_tbl i8
1124+
on i8.q2 = 456
1125+
right join text_tbl t2
1126+
on t1.f1 = 'doh!'
1127+
left join int4_tbl i4
1128+
on i8.q1 = i4.f1;
1129+
11111130
--
11121131
-- test ability to push constants through outer join clauses
11131132
--

0 commit comments

Comments
 (0)