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

Commit7a844c7

Browse files
committed
Fix joinclause removal logic to cope with cloned clauses.
When we're deleting a no-op LEFT JOIN from the query, we must removethe join's joinclauses from surviving relations' joininfo lists.The invention of "cloned" clauses in2489d76 broke the logic forthat; it'd fail to remove clones that include OJ relids outside thedoomed join's min relid sets, which could happen if that join waspreviously discovered to commute with some other join.This accidentally failed to cause problems in the majority of cases,because we'd never decide that such a cloned clause was evaluatable atany surviving join. However, Richard Guo discovered a case where thatdid happen, leading to "no relation entry for relid" errors later.Also, adding assertions that a non-removed clause contains no Vars fromthe doomed join exposes that there are quite a few existing regressiontest cases where the problem happens but is accidentally not exposed.The fix for this is just to include the target join's commute_above_rand commute_below_l sets in the relid set we test against whendeciding whether a join clause is "pushed down" and thus notremovable.While at it, do a little refactoring: the join's relid set can becomputed inside remove_rel_from_query rather than in the caller.Patch by me; thanks to Richard Guo for review.Discussion:https://postgr.es/m/CAMbWs4_PHrRqTKDNnTRsxxQy6BtYCVKsgXm1_gdN2yQ=kmcO5g@mail.gmail.com
1 parentf4a9422 commit7a844c7

File tree

3 files changed

+82
-31
lines changed

3 files changed

+82
-31
lines changed

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

Lines changed: 60 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,8 @@
3535

3636
/* local functions */
3737
staticbooljoin_is_removable(PlannerInfo*root,SpecialJoinInfo*sjinfo);
38-
staticvoidremove_rel_from_query(PlannerInfo*root,intrelid,intojrelid,
39-
Relidsjoinrelids);
38+
staticvoidremove_rel_from_query(PlannerInfo*root,intrelid,
39+
SpecialJoinInfo*sjinfo);
4040
staticvoidremove_rel_from_restrictinfo(RestrictInfo*rinfo,
4141
intrelid,intojrelid);
4242
staticList*remove_rel_from_joinlist(List*joinlist,intrelid,int*nremoved);
@@ -73,7 +73,6 @@ remove_useless_joins(PlannerInfo *root, List *joinlist)
7373
foreach(lc,root->join_info_list)
7474
{
7575
SpecialJoinInfo*sjinfo= (SpecialJoinInfo*)lfirst(lc);
76-
Relidsjoinrelids;
7776
intinnerrelid;
7877
intnremoved;
7978

@@ -88,12 +87,7 @@ remove_useless_joins(PlannerInfo *root, List *joinlist)
8887
*/
8988
innerrelid=bms_singleton_member(sjinfo->min_righthand);
9089

91-
/* Compute the relid set for the join we are considering */
92-
joinrelids=bms_union(sjinfo->min_lefthand,sjinfo->min_righthand);
93-
if (sjinfo->ojrelid!=0)
94-
joinrelids=bms_add_member(joinrelids,sjinfo->ojrelid);
95-
96-
remove_rel_from_query(root,innerrelid,sjinfo->ojrelid,joinrelids);
90+
remove_rel_from_query(root,innerrelid,sjinfo);
9791

9892
/* We verify that exactly one reference gets removed from joinlist */
9993
nremoved=0;
@@ -324,21 +318,29 @@ join_is_removable(PlannerInfo *root, SpecialJoinInfo *sjinfo)
324318

325319

326320
/*
327-
* Remove the target relid from the planner's data structures, having
328-
* determined that there is no need to include it in the query.
321+
* Remove the target relid and references to the target join from the
322+
* planner's data structures, having determined that there is no need
323+
* to include them in the query.
329324
*
330325
* We are not terribly thorough here. We only bother to update parts of
331326
* the planner's data structures that will actually be consulted later.
332327
*/
333328
staticvoid
334-
remove_rel_from_query(PlannerInfo*root,intrelid,intojrelid,
335-
Relidsjoinrelids)
329+
remove_rel_from_query(PlannerInfo*root,intrelid,SpecialJoinInfo*sjinfo)
336330
{
337331
RelOptInfo*rel=find_base_rel(root,relid);
332+
intojrelid=sjinfo->ojrelid;
333+
Relidsjoinrelids;
334+
Relidsjoin_plus_commute;
338335
List*joininfos;
339336
Indexrti;
340337
ListCell*l;
341338

339+
/* Compute the relid set for the join we are considering */
340+
joinrelids=bms_union(sjinfo->min_lefthand,sjinfo->min_righthand);
341+
Assert(ojrelid!=0);
342+
joinrelids=bms_add_member(joinrelids,ojrelid);
343+
342344
/*
343345
* Remove references to the rel from other baserels' attr_needed arrays.
344346
*/
@@ -386,21 +388,21 @@ remove_rel_from_query(PlannerInfo *root, int relid, int ojrelid,
386388
*/
387389
foreach(l,root->join_info_list)
388390
{
389-
SpecialJoinInfo*sjinfo= (SpecialJoinInfo*)lfirst(l);
390-
391-
sjinfo->min_lefthand=bms_del_member(sjinfo->min_lefthand,relid);
392-
sjinfo->min_righthand=bms_del_member(sjinfo->min_righthand,relid);
393-
sjinfo->syn_lefthand=bms_del_member(sjinfo->syn_lefthand,relid);
394-
sjinfo->syn_righthand=bms_del_member(sjinfo->syn_righthand,relid);
395-
sjinfo->min_lefthand=bms_del_member(sjinfo->min_lefthand,ojrelid);
396-
sjinfo->min_righthand=bms_del_member(sjinfo->min_righthand,ojrelid);
397-
sjinfo->syn_lefthand=bms_del_member(sjinfo->syn_lefthand,ojrelid);
398-
sjinfo->syn_righthand=bms_del_member(sjinfo->syn_righthand,ojrelid);
391+
SpecialJoinInfo*sjinf= (SpecialJoinInfo*)lfirst(l);
392+
393+
sjinf->min_lefthand=bms_del_member(sjinf->min_lefthand,relid);
394+
sjinf->min_righthand=bms_del_member(sjinf->min_righthand,relid);
395+
sjinf->syn_lefthand=bms_del_member(sjinf->syn_lefthand,relid);
396+
sjinf->syn_righthand=bms_del_member(sjinf->syn_righthand,relid);
397+
sjinf->min_lefthand=bms_del_member(sjinf->min_lefthand,ojrelid);
398+
sjinf->min_righthand=bms_del_member(sjinf->min_righthand,ojrelid);
399+
sjinf->syn_lefthand=bms_del_member(sjinf->syn_lefthand,ojrelid);
400+
sjinf->syn_righthand=bms_del_member(sjinf->syn_righthand,ojrelid);
399401
/* relid cannot appear in these fields, but ojrelid can: */
400-
sjinfo->commute_above_l=bms_del_member(sjinfo->commute_above_l,ojrelid);
401-
sjinfo->commute_above_r=bms_del_member(sjinfo->commute_above_r,ojrelid);
402-
sjinfo->commute_below_l=bms_del_member(sjinfo->commute_below_l,ojrelid);
403-
sjinfo->commute_below_r=bms_del_member(sjinfo->commute_below_r,ojrelid);
402+
sjinf->commute_above_l=bms_del_member(sjinf->commute_above_l,ojrelid);
403+
sjinf->commute_above_r=bms_del_member(sjinf->commute_above_r,ojrelid);
404+
sjinf->commute_below_l=bms_del_member(sjinf->commute_below_l,ojrelid);
405+
sjinf->commute_below_r=bms_del_member(sjinf->commute_below_r,ojrelid);
404406
}
405407

406408
/*
@@ -456,6 +458,18 @@ remove_rel_from_query(PlannerInfo *root, int relid, int ojrelid,
456458
* just discard them, though. Only quals that logically belonged to the
457459
* outer join being discarded should be removed from the query.
458460
*
461+
* We might encounter a qual that is a clone of a deletable qual with some
462+
* outer-join relids added (see deconstruct_distribute_oj_quals). To
463+
* ensure we get rid of such clones as well, add the relids of all OJs
464+
* commutable with this one to the set we test against for
465+
* pushed-down-ness.
466+
*/
467+
join_plus_commute=bms_union(joinrelids,
468+
sjinfo->commute_above_r);
469+
join_plus_commute=bms_add_members(join_plus_commute,
470+
sjinfo->commute_below_l);
471+
472+
/*
459473
* We must make a copy of the rel's old joininfo list before starting the
460474
* loop, because otherwise remove_join_clause_from_rels would destroy the
461475
* list while we're scanning it.
@@ -467,15 +481,30 @@ remove_rel_from_query(PlannerInfo *root, int relid, int ojrelid,
467481

468482
remove_join_clause_from_rels(root,rinfo,rinfo->required_relids);
469483

470-
if (RINFO_IS_PUSHED_DOWN(rinfo,joinrelids))
484+
if (RINFO_IS_PUSHED_DOWN(rinfo,join_plus_commute))
471485
{
472486
/*
473487
* There might be references to relid or ojrelid in the
474-
* RestrictInfo, as a consequence of PHVs having ph_eval_at sets
475-
* that include those. We already checked above that any such PHV
476-
* is safe, so we can just drop those references.
488+
* RestrictInfo's relid sets, as a consequence of PHVs having had
489+
* ph_eval_at sets that include those. We already checked above
490+
* that any such PHV is safe (and updated its ph_eval_at), so we
491+
* can just drop those references.
477492
*/
478493
remove_rel_from_restrictinfo(rinfo,relid,ojrelid);
494+
495+
/*
496+
* Cross-check that the clause itself does not reference the
497+
* target rel or join.
498+
*/
499+
#ifdefUSE_ASSERT_CHECKING
500+
{
501+
Relidsclause_varnos=pull_varnos(root,
502+
(Node*)rinfo->clause);
503+
504+
Assert(!bms_is_member(relid,clause_varnos));
505+
Assert(!bms_is_member(ojrelid,clause_varnos));
506+
}
507+
#endif
479508
/* Now throw it back into the joininfo lists */
480509
distribute_restrictinfo_to_rels(root,rinfo);
481510
}

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5320,6 +5320,22 @@ select a1.id from
53205320
Seq Scan on a a1
53215321
(1 row)
53225322

5323+
explain (costs off)
5324+
select 1 from a t1
5325+
left join a t2 on true
5326+
inner join a t3 on true
5327+
left join a t4 on t2.id = t4.id and t2.id = t3.id;
5328+
QUERY PLAN
5329+
------------------------------------
5330+
Nested Loop
5331+
-> Nested Loop Left Join
5332+
-> Seq Scan on a t1
5333+
-> Materialize
5334+
-> Seq Scan on a t2
5335+
-> Materialize
5336+
-> Seq Scan on a t3
5337+
(7 rows)
5338+
53235339
-- another example (bug #17781)
53245340
explain (costs off)
53255341
select ss1.f1

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1891,6 +1891,12 @@ select a1.id from
18911891
(a a3left join a a4ona3.id=a4.id)
18921892
ona2.id=a3.id;
18931893

1894+
explain (costs off)
1895+
select1from a t1
1896+
left join a t2on true
1897+
inner join a t3on true
1898+
left join a t4ont2.id=t4.idandt2.id=t3.id;
1899+
18941900
-- another example (bug #17781)
18951901
explain (costs off)
18961902
selectss1.f1

0 commit comments

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