Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Commitb2d0e13

Browse files
committed
Fix over-optimistic updating of info about commutable outer joins.
make_outerjoininfo was set up to update SpecialJoinInfo'scommute_below, commute_above_l, commute_above_r fields as soon asit found a pair of outer joins that look like they can commute.However, this decision could be negated later in the same loop dueto finding an intermediate outer join that prevents commutation.That left us with commute_xxx fields that were contradictory to thejoin order restrictions expressed in min_lefthand/min_righthand.The latter fields would keep us from actually choosing a bad joinorder; but the inconsistent commute_xxx fields could bollix detailssuch as the varnullingrels values created for intermediate joinrelation targetlists, ending in an assertion failure in setrefs.c.To fix, wait till the end of make_outerjoininfo where we haveaccurate values for min_lefthand/min_righthand, and then insertonly relids not present in those sets into the commute_xxx fields.Per SQLSmith testing by Robins Tharakan. Note that while Robinsbisected the failure to commitb448f1c, it's really the fault of2489d76. The outerjoin_delayed logic removed in the later commitwas keeping us from deciding that troublesome join pairs commute,at least in the specific example seen here.Discussion:https://postgr.es/m/CAEP4nAyAORgE8K_RHSmvWbE9UaChhjbEL1RrDU3neePwwRUB=A@mail.gmail.com
1 parent9f452fe commitb2d0e13

File tree

3 files changed

+99
-16
lines changed

3 files changed

+99
-16
lines changed

‎src/backend/optimizer/plan/initsplan.c

Lines changed: 58 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1358,6 +1358,8 @@ make_outerjoininfo(PlannerInfo *root,
13581358
Relidsstrict_relids;
13591359
Relidsmin_lefthand;
13601360
Relidsmin_righthand;
1361+
Relidscommute_below_l;
1362+
Relidscommute_below_r;
13611363
ListCell*l;
13621364

13631365
/*
@@ -1445,7 +1447,14 @@ make_outerjoininfo(PlannerInfo *root,
14451447

14461448
/*
14471449
* Now check previous outer joins for ordering restrictions.
1450+
*
1451+
* commute_below_l and commute_below_r accumulate the relids of lower
1452+
* outer joins that we think this one can commute with. These decisions
1453+
* are just tentative within this loop, since we might find an
1454+
* intermediate outer join that prevents commutation. Surviving relids
1455+
* will get merged into the SpecialJoinInfo structs afterwards.
14481456
*/
1457+
commute_below_l=commute_below_r=NULL;
14491458
foreach(l,root->join_info_list)
14501459
{
14511460
SpecialJoinInfo*otherinfo= (SpecialJoinInfo*)lfirst(l);
@@ -1458,16 +1467,16 @@ make_outerjoininfo(PlannerInfo *root,
14581467
*/
14591468
if (otherinfo->jointype==JOIN_FULL)
14601469
{
1470+
Assert(otherinfo->ojrelid!=0);
14611471
if (bms_overlap(left_rels,otherinfo->syn_lefthand)||
14621472
bms_overlap(left_rels,otherinfo->syn_righthand))
14631473
{
14641474
min_lefthand=bms_add_members(min_lefthand,
14651475
otherinfo->syn_lefthand);
14661476
min_lefthand=bms_add_members(min_lefthand,
14671477
otherinfo->syn_righthand);
1468-
if (otherinfo->ojrelid!=0)
1469-
min_lefthand=bms_add_member(min_lefthand,
1470-
otherinfo->ojrelid);
1478+
min_lefthand=bms_add_member(min_lefthand,
1479+
otherinfo->ojrelid);
14711480
}
14721481
if (bms_overlap(right_rels,otherinfo->syn_lefthand)||
14731482
bms_overlap(right_rels,otherinfo->syn_righthand))
@@ -1476,9 +1485,8 @@ make_outerjoininfo(PlannerInfo *root,
14761485
otherinfo->syn_lefthand);
14771486
min_righthand=bms_add_members(min_righthand,
14781487
otherinfo->syn_righthand);
1479-
if (otherinfo->ojrelid!=0)
1480-
min_righthand=bms_add_member(min_righthand,
1481-
otherinfo->ojrelid);
1488+
min_righthand=bms_add_member(min_righthand,
1489+
otherinfo->ojrelid);
14821490
}
14831491
/* Needn't do anything else with the full join */
14841492
continue;
@@ -1536,11 +1544,9 @@ make_outerjoininfo(PlannerInfo *root,
15361544
{
15371545
/* Identity 3 applies, so remove the ordering restriction */
15381546
min_lefthand=bms_del_member(min_lefthand,otherinfo->ojrelid);
1539-
/* Add commutability markers to both SpecialJoinInfos */
1540-
otherinfo->commute_above_l=
1541-
bms_add_member(otherinfo->commute_above_l,ojrelid);
1542-
sjinfo->commute_below=
1543-
bms_add_member(sjinfo->commute_below,otherinfo->ojrelid);
1547+
/* Record the (still tentative) commutability relationship */
1548+
commute_below_l=
1549+
bms_add_member(commute_below_l,otherinfo->ojrelid);
15441550
}
15451551
}
15461552

@@ -1589,11 +1595,9 @@ make_outerjoininfo(PlannerInfo *root,
15891595
/* Identity 3 applies, so remove the ordering restriction */
15901596
min_righthand=bms_del_member(min_righthand,
15911597
otherinfo->ojrelid);
1592-
/* Add commutability markers to both SpecialJoinInfos */
1593-
otherinfo->commute_above_r=
1594-
bms_add_member(otherinfo->commute_above_r,ojrelid);
1595-
sjinfo->commute_below=
1596-
bms_add_member(sjinfo->commute_below,otherinfo->ojrelid);
1598+
/* Record the (still tentative) commutability relationship */
1599+
commute_below_r=
1600+
bms_add_member(commute_below_r,otherinfo->ojrelid);
15971601
}
15981602
}
15991603
}
@@ -1639,6 +1643,44 @@ make_outerjoininfo(PlannerInfo *root,
16391643
sjinfo->min_lefthand=min_lefthand;
16401644
sjinfo->min_righthand=min_righthand;
16411645

1646+
/*
1647+
* Now that we've identified the correct min_lefthand and min_righthand,
1648+
* any commute_below_l or commute_below_r relids that have not gotten
1649+
* added back into those sets (due to intervening outer joins) are indeed
1650+
* commutable with this one. Update the derived data in the
1651+
* SpecialJoinInfos.
1652+
*/
1653+
if (commute_below_l||commute_below_r)
1654+
{
1655+
Relidscommute_below;
1656+
1657+
/*
1658+
* Delete any subsequently-added-back relids (this is easier than
1659+
* maintaining commute_below_l/r precisely through all the above).
1660+
*/
1661+
commute_below_l=bms_del_members(commute_below_l,min_lefthand);
1662+
commute_below_r=bms_del_members(commute_below_r,min_righthand);
1663+
1664+
/* Anything left? */
1665+
commute_below=bms_union(commute_below_l,commute_below_r);
1666+
if (!bms_is_empty(commute_below))
1667+
{
1668+
/* Yup, so we must update the data structures */
1669+
sjinfo->commute_below=commute_below;
1670+
foreach(l,root->join_info_list)
1671+
{
1672+
SpecialJoinInfo*otherinfo= (SpecialJoinInfo*)lfirst(l);
1673+
1674+
if (bms_is_member(otherinfo->ojrelid,commute_below_l))
1675+
otherinfo->commute_above_l=
1676+
bms_add_member(otherinfo->commute_above_l,ojrelid);
1677+
elseif (bms_is_member(otherinfo->ojrelid,commute_below_r))
1678+
otherinfo->commute_above_r=
1679+
bms_add_member(otherinfo->commute_above_r,ojrelid);
1680+
}
1681+
}
1682+
}
1683+
16421684
returnsjinfo;
16431685
}
16441686

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4469,6 +4469,36 @@ left join
44694469
One-Time Filter: false
44704470
(5 rows)
44714471

4472+
-- check handling of apparently-commutable outer joins with non-commutable
4473+
-- joins between them
4474+
explain (costs off)
4475+
select 1 from
4476+
int4_tbl i4
4477+
left join int8_tbl i8 on i4.f1 is not null
4478+
left join (select 1 as a) ss1 on null
4479+
join int4_tbl i42 on ss1.a is null or i8.q1 <> i8.q2
4480+
right join (select 2 as b) ss2
4481+
on ss2.b < i4.f1;
4482+
QUERY PLAN
4483+
-----------------------------------------------------------
4484+
Nested Loop Left Join
4485+
-> Result
4486+
-> Nested Loop
4487+
-> Nested Loop Left Join
4488+
Join Filter: NULL::boolean
4489+
Filter: (((1) IS NULL) OR (i8.q1 <> i8.q2))
4490+
-> Nested Loop Left Join
4491+
Join Filter: (i4.f1 IS NOT NULL)
4492+
-> Seq Scan on int4_tbl i4
4493+
Filter: (2 < f1)
4494+
-> Materialize
4495+
-> Seq Scan on int8_tbl i8
4496+
-> Result
4497+
One-Time Filter: false
4498+
-> Materialize
4499+
-> Seq Scan on int4_tbl i42
4500+
(16 rows)
4501+
44724502
--
44734503
-- test for appropriate join order in the presence of lateral references
44744504
--

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1545,6 +1545,17 @@ left join
15451545
wherec.relkind='r'
15461546
) ss2on false;
15471547

1548+
-- check handling of apparently-commutable outer joins with non-commutable
1549+
-- joins between them
1550+
explain (costs off)
1551+
select1from
1552+
int4_tbl i4
1553+
left join int8_tbl i8oni4.f1is not null
1554+
left join (select1as a) ss1onnull
1555+
join int4_tbl i42onss1.a isnullori8.q1<>i8.q2
1556+
right join (select2as b) ss2
1557+
onss2.b<i4.f1;
1558+
15481559
--
15491560
-- test for appropriate join order in the presence of lateral references
15501561
--

0 commit comments

Comments
 (0)

[8]ページ先頭

©2009-2025 Movatter.jp