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

Commit11086f2

Browse files
committed
Repair planner bug introduced in 8.2 by ability to rearrange outer joins:
in cases where a sub-SELECT inserts a WHERE clause between two outer joins,that clause may prevent us from re-ordering the two outer joins. The codewas considering only the joins' own ON-conditions in determining reorderingsafety, which is not good enough. Add a "delay_upper_joins" flag toOuterJoinInfo to flag that we have detected such a clause and higher-levelouter joins shouldn't be permitted to commute with this one. (This mightseem overly coarse, but given the current rules for OJ reordering, it'ssufficient AFAICT.)The failure case is actually pretty narrow: it needs a WHERE clause withinthe RHS of a left join that checks the RHS of a lower left join, but is notstrict for that RHS (else we'd have simplified the lower join to a plainjoin). Even then no failure will be manifest unless the planner chooses torearrange the join order.Per bug report from Adam Terrey.
1 parent0f6e2d0 commit11086f2

File tree

8 files changed

+138
-13
lines changed

8 files changed

+138
-13
lines changed

‎src/backend/nodes/copyfuncs.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
* Portions Copyright (c) 1994, Regents of the University of California
1616
*
1717
* IDENTIFICATION
18-
* $PostgreSQL: pgsql/src/backend/nodes/copyfuncs.c,v 1.375 2007/04/27 22:05:47 tgl Exp $
18+
* $PostgreSQL: pgsql/src/backend/nodes/copyfuncs.c,v 1.376 2007/05/22 23:23:55 tgl Exp $
1919
*
2020
*-------------------------------------------------------------------------
2121
*/
@@ -1425,6 +1425,7 @@ _copyOuterJoinInfo(OuterJoinInfo *from)
14251425
COPY_BITMAPSET_FIELD(min_righthand);
14261426
COPY_SCALAR_FIELD(is_full_join);
14271427
COPY_SCALAR_FIELD(lhs_strict);
1428+
COPY_SCALAR_FIELD(delay_upper_joins);
14281429

14291430
returnnewnode;
14301431
}

‎src/backend/nodes/equalfuncs.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
* Portions Copyright (c) 1994, Regents of the University of California
1919
*
2020
* IDENTIFICATION
21-
* $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.306 2007/04/27 22:05:47 tgl Exp $
21+
* $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.307 2007/05/22 23:23:56 tgl Exp $
2222
*
2323
*-------------------------------------------------------------------------
2424
*/
@@ -680,6 +680,7 @@ _equalOuterJoinInfo(OuterJoinInfo *a, OuterJoinInfo *b)
680680
COMPARE_BITMAPSET_FIELD(min_righthand);
681681
COMPARE_SCALAR_FIELD(is_full_join);
682682
COMPARE_SCALAR_FIELD(lhs_strict);
683+
COMPARE_SCALAR_FIELD(delay_upper_joins);
683684

684685
return true;
685686
}

‎src/backend/nodes/outfuncs.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/nodes/outfuncs.c,v 1.307 2007/05/2201:40:33 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/nodes/outfuncs.c,v 1.308 2007/05/2223:23:56 tgl Exp $
1212
*
1313
* NOTES
1414
* Every node type that can appear in stored rules' parsetrees *must*
@@ -1453,6 +1453,7 @@ _outOuterJoinInfo(StringInfo str, OuterJoinInfo *node)
14531453
WRITE_BITMAPSET_FIELD(min_righthand);
14541454
WRITE_BOOL_FIELD(is_full_join);
14551455
WRITE_BOOL_FIELD(lhs_strict);
1456+
WRITE_BOOL_FIELD(delay_upper_joins);
14561457
}
14571458

14581459
staticvoid

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

Lines changed: 45 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/optimizer/plan/initsplan.c,v 1.131 2007/02/16 20:57:19 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/optimizer/plan/initsplan.c,v 1.132 2007/05/22 23:23:56 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -48,7 +48,8 @@ static void distribute_qual_to_rels(PlannerInfo *root, Node *clause,
4848
Relidsqualscope,
4949
Relidsojscope,
5050
Relidsouterjoin_nonnullable);
51-
staticboolcheck_outerjoin_delay(PlannerInfo*root,Relids*relids_p);
51+
staticboolcheck_outerjoin_delay(PlannerInfo*root,Relids*relids_p,
52+
boolis_pushed_down);
5253
staticvoidcheck_mergejoinable(RestrictInfo*restrictinfo);
5354
staticvoidcheck_hashjoinable(RestrictInfo*restrictinfo);
5455

@@ -492,6 +493,9 @@ make_outerjoininfo(PlannerInfo *root,
492493
errmsg("SELECT FOR UPDATE/SHARE cannot be applied to the nullable side of an outer join")));
493494
}
494495

496+
/* this always starts out false */
497+
ojinfo->delay_upper_joins= false;
498+
495499
/* If it's a full join, no need to be very smart */
496500
ojinfo->is_full_join=is_full_join;
497501
if (is_full_join)
@@ -561,10 +565,21 @@ make_outerjoininfo(PlannerInfo *root,
561565
* lower join's RHS and the lower OJ's join condition is strict, we
562566
* can interchange the ordering of the two OJs, so exclude the lower
563567
* RHS from our min_righthand.
568+
*
569+
* Here, we have to consider that "our join condition" includes
570+
* any clauses that syntactically appeared above the lower OJ and
571+
* below ours; those are equivalent to degenerate clauses in our
572+
* OJ and must be treated as such. Such clauses obviously can't
573+
* reference our LHS, and they must be non-strict for the lower OJ's
574+
* RHS (else reduce_outer_joins would have reduced the lower OJ to
575+
* a plain join). Hence the other ways in which we handle clauses
576+
* within our join condition are not affected by them. The net
577+
* effect is therefore sufficiently represented by the
578+
* delay_upper_joins flag saved for us by check_outerjoin_delay.
564579
*/
565580
if (bms_overlap(ojinfo->min_righthand,otherinfo->min_righthand)&&
566581
!bms_overlap(clause_relids,otherinfo->min_righthand)&&
567-
otherinfo->lhs_strict)
582+
otherinfo->lhs_strict&& !otherinfo->delay_upper_joins)
568583
{
569584
ojinfo->min_righthand=bms_del_members(ojinfo->min_righthand,
570585
otherinfo->min_righthand);
@@ -749,7 +764,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
749764
* clauses.
750765
*/
751766
maybe_equivalence= false;
752-
maybe_outer_join= !check_outerjoin_delay(root,&relids);
767+
maybe_outer_join= !check_outerjoin_delay(root,&relids, false);
753768

754769
/*
755770
* Now force the qual to be evaluated exactly at the level of joining
@@ -776,7 +791,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
776791
is_pushed_down= true;
777792

778793
/* Check to see if must be delayed by outer join */
779-
outerjoin_delayed=check_outerjoin_delay(root,&relids);
794+
outerjoin_delayed=check_outerjoin_delay(root,&relids, true);
780795

781796
if (outerjoin_delayed)
782797
{
@@ -918,10 +933,13 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
918933
/*
919934
* check_outerjoin_delay
920935
*Detect whether a qual referencing the given relids must be delayed
921-
*in application due to the presence of a lower outer join.
936+
*in application due to the presence of a lower outer join, and/or
937+
*may force extra delay of higher-level outer joins.
922938
*
923-
* If so, add relids to *relids_p to reflect the lowest safe level for
924-
* evaluating the qual, and return TRUE.
939+
* If the qual must be delayed, add relids to *relids_p to reflect the lowest
940+
* safe level for evaluating the qual, and return TRUE. Any extra delay for
941+
* higher-level joins is reflected by setting delay_upper_joins to TRUE in
942+
* OuterJoinInfo structs.
925943
*
926944
* For an is_pushed_down qual, we can evaluate the qual as soon as (1) we have
927945
* all the rels it mentions, and (2) we are at or above any outer joins that
@@ -946,9 +964,23 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
946964
* For a non-pushed-down qual, this isn't going to determine where we place the
947965
* qual, but we need to determine outerjoin_delayed anyway so we can decide
948966
* whether the qual is potentially useful for equivalence deductions.
967+
*
968+
* Lastly, a pushed-down qual that references the nullable side of any current
969+
* oj_info_list member and has to be evaluated above that OJ (because its
970+
* required relids overlap the LHS too) causes that OJ's delay_upper_joins
971+
* flag to be set TRUE. This will prevent any higher-level OJs from
972+
* being interchanged with that OJ, which would result in not having any
973+
* correct place to evaluate the qual. (The case we care about here is a
974+
* sub-select WHERE clause within the RHS of some outer join. The WHERE
975+
* clause must effectively be treated as a degenerate clause of that outer
976+
* join's condition. Rather than trying to match such clauses with joins
977+
* directly, we set delay_upper_joins here, and when the upper outer join
978+
* is processed by make_outerjoininfo, it will refrain from allowing the
979+
* two OJs to commute.)
949980
*/
950981
staticbool
951-
check_outerjoin_delay(PlannerInfo*root,Relids*relids_p)
982+
check_outerjoin_delay(PlannerInfo*root,Relids*relids_p,
983+
boolis_pushed_down)
952984
{
953985
Relidsrelids=*relids_p;
954986
boolouterjoin_delayed;
@@ -979,6 +1011,10 @@ check_outerjoin_delay(PlannerInfo *root, Relids *relids_p)
9791011
/* we'll need another iteration */
9801012
found_some= true;
9811013
}
1014+
/* set delay_upper_joins if needed */
1015+
if (is_pushed_down&& !ojinfo->is_full_join&&
1016+
bms_overlap(relids,ojinfo->min_lefthand))
1017+
ojinfo->delay_upper_joins= true;
9821018
}
9831019
}
9841020
}while (found_some);

‎src/include/nodes/relation.h

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1996-2007, PostgreSQL Global Development Group
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $PostgreSQL: pgsql/src/include/nodes/relation.h,v 1.142 2007/05/2201:40:33 tgl Exp $
10+
* $PostgreSQL: pgsql/src/include/nodes/relation.h,v 1.143 2007/05/2223:23:57 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -1042,6 +1042,12 @@ typedef struct InnerIndexscanInfo
10421042
* It is not valid for either min_lefthand or min_righthand to be empty sets;
10431043
* if they were, this would break the logic that enforces join order.
10441044
*
1045+
* delay_upper_joins is set TRUE if we detect a pushed-down clause that has
1046+
* to be evaluated after this join is formed (because it references the RHS).
1047+
* Any outer joins that have such a clause and this join in their RHS cannot
1048+
* commute with this join, because that would leave noplace to check the
1049+
* pushed-down clause. (We don't track this for FULL JOINs, either.)
1050+
*
10451051
* Note: OuterJoinInfo directly represents only LEFT JOIN and FULL JOIN;
10461052
* RIGHT JOIN is handled by switching the inputs to make it a LEFT JOIN.
10471053
* We make an OuterJoinInfo for FULL JOINs even though there is no flexibility
@@ -1055,6 +1061,7 @@ typedef struct OuterJoinInfo
10551061
Relidsmin_righthand;/* base relids in minimum RHS for join */
10561062
boolis_full_join;/* it's a FULL OUTER JOIN */
10571063
boollhs_strict;/* joinclause is strict for some LHS rel */
1064+
booldelay_upper_joins;/* can't commute with upper RHS */
10581065
}OuterJoinInfo;
10591066

10601067
/*

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2223,3 +2223,30 @@ select tt1.*, tt2.* from tt2 right join tt1 on tt1.joincol = tt2.joincol;
22232223
2 | | |
22242224
(3 rows)
22252225

2226+
reset enable_hashjoin;
2227+
reset enable_nestloop;
2228+
--
2229+
-- regression test for 8.2 bug with improper re-ordering of left joins
2230+
--
2231+
create temp table tt3(f1 int, f2 text);
2232+
insert into tt3 select x, repeat('xyzzy', 100) from generate_series(1,10000) x;
2233+
create index tt3i on tt3(f1);
2234+
analyze tt3;
2235+
create temp table tt4(f1 int);
2236+
insert into tt4 values (0),(1),(9999);
2237+
analyze tt4;
2238+
SELECT a.f1
2239+
FROM tt4 a
2240+
LEFT JOIN (
2241+
SELECT b.f1
2242+
FROM tt3 b LEFT JOIN tt3 c ON (b.f1 = c.f1)
2243+
WHERE c.f1 IS NULL
2244+
) AS d ON (a.f1 = d.f1)
2245+
WHERE d.f1 IS NULL;
2246+
f1
2247+
------
2248+
0
2249+
1
2250+
9999
2251+
(3 rows)
2252+

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2223,3 +2223,30 @@ select tt1.*, tt2.* from tt2 right join tt1 on tt1.joincol = tt2.joincol;
22232223
2 | | |
22242224
(3 rows)
22252225

2226+
reset enable_hashjoin;
2227+
reset enable_nestloop;
2228+
--
2229+
-- regression test for 8.2 bug with improper re-ordering of left joins
2230+
--
2231+
create temp table tt3(f1 int, f2 text);
2232+
insert into tt3 select x, repeat('xyzzy', 100) from generate_series(1,10000) x;
2233+
create index tt3i on tt3(f1);
2234+
analyze tt3;
2235+
create temp table tt4(f1 int);
2236+
insert into tt4 values (0),(1),(9999);
2237+
analyze tt4;
2238+
SELECT a.f1
2239+
FROM tt4 a
2240+
LEFT JOIN (
2241+
SELECT b.f1
2242+
FROM tt3 b LEFT JOIN tt3 c ON (b.f1 = c.f1)
2243+
WHERE c.f1 IS NULL
2244+
) AS d ON (a.f1 = d.f1)
2245+
WHERE d.f1 IS NULL;
2246+
f1
2247+
------
2248+
0
2249+
1
2250+
9999
2251+
(3 rows)
2252+

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -399,3 +399,28 @@ set enable_nestloop to off;
399399
select tt1.*, tt2.*from tt1left join tt2ontt1.joincol=tt2.joincol;
400400

401401
select tt1.*, tt2.*from tt2right join tt1ontt1.joincol=tt2.joincol;
402+
403+
reset enable_hashjoin;
404+
reset enable_nestloop;
405+
406+
--
407+
-- regression test for 8.2 bug with improper re-ordering of left joins
408+
--
409+
410+
create temp table tt3(f1int, f2text);
411+
insert into tt3select x, repeat('xyzzy',100)from generate_series(1,10000) x;
412+
createindextt3ion tt3(f1);
413+
analyze tt3;
414+
415+
create temp table tt4(f1int);
416+
insert into tt4values (0),(1),(9999);
417+
analyze tt4;
418+
419+
SELECTa.f1
420+
FROM tt4 a
421+
LEFT JOIN (
422+
SELECTb.f1
423+
FROM tt3 bLEFT JOIN tt3 cON (b.f1=c.f1)
424+
WHEREc.f1 ISNULL
425+
)AS dON (a.f1=d.f1)
426+
WHEREd.f1 ISNULL;

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp