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

Commit8b6294c

Browse files
committed
Change more places to be less trusting of RestrictInfo.is_pushed_down.
On further reflection, commite5d8399 didn't go far enough: pretty mucheverywhere in the planner that examines a clause's is_pushed_down flagought to be changed to use the more complicated behavior where we alsocheck the clause's required_relids. Otherwise we could make incorrectdecisions about whether, say, a clause is safe to use as a hash clause.Some (many?) of these places are safe as-is, either because they arenever reached while considering a parameterized path, or because thereare additional checks that would reject a pushed-down clause anyway.However, it seems smarter to just code them all the same way ratherthan rely on easily-broken reasoning of that sort.In support of that, invent a new macro RINFO_IS_PUSHED_DOWN that shouldbe used in place of direct tests on the is_pushed_down flag.Like the previous patch, back-patch to all supported branches.Discussion:https://postgr.es/m/f8128b11-c5bf-3539-48cd-234178b2314d@proxel.se
1 parent68fab04 commit8b6294c

File tree

8 files changed

+57
-34
lines changed

8 files changed

+57
-34
lines changed

‎contrib/postgres_fdw/postgres_fdw.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4127,7 +4127,8 @@ foreign_join_ok(PlannerInfo *root, RelOptInfo *joinrel, JoinType jointype,
41274127
boolis_remote_clause=is_foreign_expr(root,joinrel,
41284128
rinfo->clause);
41294129

4130-
if (IS_OUTER_JOIN(jointype)&& !rinfo->is_pushed_down)
4130+
if (IS_OUTER_JOIN(jointype)&&
4131+
!RINFO_IS_PUSHED_DOWN(rinfo,joinrel->relids))
41314132
{
41324133
if (!is_remote_clause)
41334134
return false;

‎src/backend/optimizer/path/costsize.c

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,7 @@ static bool has_indexed_join_quals(NestPath *joinpath);
148148
staticdoubleapprox_tuple_count(PlannerInfo*root,JoinPath*path,
149149
List*quals);
150150
staticdoublecalc_joinrel_size_estimate(PlannerInfo*root,
151+
RelOptInfo*joinrel,
151152
RelOptInfo*outer_rel,
152153
RelOptInfo*inner_rel,
153154
doubleouter_rows,
@@ -3776,12 +3777,14 @@ compute_semi_anti_join_factors(PlannerInfo *root,
37763777
*/
37773778
if (IS_OUTER_JOIN(jointype))
37783779
{
3780+
Relidsjoinrelids=bms_union(outerrel->relids,innerrel->relids);
3781+
37793782
joinquals=NIL;
37803783
foreach(l,restrictlist)
37813784
{
37823785
RestrictInfo*rinfo=lfirst_node(RestrictInfo,l);
37833786

3784-
if (!rinfo->is_pushed_down)
3787+
if (!RINFO_IS_PUSHED_DOWN(rinfo,joinrelids))
37853788
joinquals=lappend(joinquals,rinfo);
37863789
}
37873790
}
@@ -4096,6 +4099,7 @@ set_joinrel_size_estimates(PlannerInfo *root, RelOptInfo *rel,
40964099
List*restrictlist)
40974100
{
40984101
rel->rows=calc_joinrel_size_estimate(root,
4102+
rel,
40994103
outer_rel,
41004104
inner_rel,
41014105
outer_rel->rows,
@@ -4138,6 +4142,7 @@ get_parameterized_joinrel_size(PlannerInfo *root, RelOptInfo *rel,
41384142
* estimate for any pair with the same parameterization.
41394143
*/
41404144
nrows=calc_joinrel_size_estimate(root,
4145+
rel,
41414146
outer_path->parent,
41424147
inner_path->parent,
41434148
outer_path->rows,
@@ -4161,6 +4166,7 @@ get_parameterized_joinrel_size(PlannerInfo *root, RelOptInfo *rel,
41614166
*/
41624167
staticdouble
41634168
calc_joinrel_size_estimate(PlannerInfo*root,
4169+
RelOptInfo*joinrel,
41644170
RelOptInfo*outer_rel,
41654171
RelOptInfo*inner_rel,
41664172
doubleouter_rows,
@@ -4213,7 +4219,7 @@ calc_joinrel_size_estimate(PlannerInfo *root,
42134219
{
42144220
RestrictInfo*rinfo=lfirst_node(RestrictInfo,l);
42154221

4216-
if (rinfo->is_pushed_down)
4222+
if (RINFO_IS_PUSHED_DOWN(rinfo,joinrel->relids))
42174223
pushedquals=lappend(pushedquals,rinfo);
42184224
else
42194225
joinquals=lappend(joinquals,rinfo);

‎src/backend/optimizer/path/joinpath.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1606,7 +1606,7 @@ hash_inner_and_outer(PlannerInfo *root,
16061606
* If processing an outer join, only use its own join clauses for
16071607
* hashing. For inner joins we need not be so picky.
16081608
*/
1609-
if (isouterjoin&&restrictinfo->is_pushed_down)
1609+
if (isouterjoin&&RINFO_IS_PUSHED_DOWN(restrictinfo,joinrel->relids))
16101610
continue;
16111611

16121612
if (!restrictinfo->can_join||
@@ -1832,7 +1832,7 @@ select_mergejoin_clauses(PlannerInfo *root,
18321832
* we don't set have_nonmergeable_joinclause here because pushed-down
18331833
* clauses will become otherquals not joinquals.)
18341834
*/
1835-
if (isouterjoin&&restrictinfo->is_pushed_down)
1835+
if (isouterjoin&&RINFO_IS_PUSHED_DOWN(restrictinfo,joinrel->relids))
18361836
continue;
18371837

18381838
/* Check that clause is a mergeable operator clause */

‎src/backend/optimizer/path/joinrels.c

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ static bool has_legal_joinclause(PlannerInfo *root, RelOptInfo *rel);
3131
staticboolis_dummy_rel(RelOptInfo*rel);
3232
staticvoidmark_dummy_rel(RelOptInfo*rel);
3333
staticboolrestriction_is_constant_false(List*restrictlist,
34+
RelOptInfo*joinrel,
3435
boolonly_pushed_down);
3536
staticvoidpopulate_joinrel_with_paths(PlannerInfo*root,RelOptInfo*rel1,
3637
RelOptInfo*rel2,RelOptInfo*joinrel,
@@ -770,7 +771,7 @@ populate_joinrel_with_paths(PlannerInfo *root, RelOptInfo *rel1,
770771
{
771772
caseJOIN_INNER:
772773
if (is_dummy_rel(rel1)||is_dummy_rel(rel2)||
773-
restriction_is_constant_false(restrictlist, false))
774+
restriction_is_constant_false(restrictlist,joinrel,false))
774775
{
775776
mark_dummy_rel(joinrel);
776777
break;
@@ -784,12 +785,12 @@ populate_joinrel_with_paths(PlannerInfo *root, RelOptInfo *rel1,
784785
break;
785786
caseJOIN_LEFT:
786787
if (is_dummy_rel(rel1)||
787-
restriction_is_constant_false(restrictlist, true))
788+
restriction_is_constant_false(restrictlist,joinrel,true))
788789
{
789790
mark_dummy_rel(joinrel);
790791
break;
791792
}
792-
if (restriction_is_constant_false(restrictlist, false)&&
793+
if (restriction_is_constant_false(restrictlist,joinrel,false)&&
793794
bms_is_subset(rel2->relids,sjinfo->syn_righthand))
794795
mark_dummy_rel(rel2);
795796
add_paths_to_joinrel(root,joinrel,rel1,rel2,
@@ -801,7 +802,7 @@ populate_joinrel_with_paths(PlannerInfo *root, RelOptInfo *rel1,
801802
break;
802803
caseJOIN_FULL:
803804
if ((is_dummy_rel(rel1)&&is_dummy_rel(rel2))||
804-
restriction_is_constant_false(restrictlist, true))
805+
restriction_is_constant_false(restrictlist,joinrel,true))
805806
{
806807
mark_dummy_rel(joinrel);
807808
break;
@@ -837,7 +838,7 @@ populate_joinrel_with_paths(PlannerInfo *root, RelOptInfo *rel1,
837838
bms_is_subset(sjinfo->min_righthand,rel2->relids))
838839
{
839840
if (is_dummy_rel(rel1)||is_dummy_rel(rel2)||
840-
restriction_is_constant_false(restrictlist, false))
841+
restriction_is_constant_false(restrictlist,joinrel,false))
841842
{
842843
mark_dummy_rel(joinrel);
843844
break;
@@ -860,7 +861,7 @@ populate_joinrel_with_paths(PlannerInfo *root, RelOptInfo *rel1,
860861
sjinfo)!=NULL)
861862
{
862863
if (is_dummy_rel(rel1)||is_dummy_rel(rel2)||
863-
restriction_is_constant_false(restrictlist, false))
864+
restriction_is_constant_false(restrictlist,joinrel,false))
864865
{
865866
mark_dummy_rel(joinrel);
866867
break;
@@ -875,12 +876,12 @@ populate_joinrel_with_paths(PlannerInfo *root, RelOptInfo *rel1,
875876
break;
876877
caseJOIN_ANTI:
877878
if (is_dummy_rel(rel1)||
878-
restriction_is_constant_false(restrictlist, true))
879+
restriction_is_constant_false(restrictlist,joinrel,true))
879880
{
880881
mark_dummy_rel(joinrel);
881882
break;
882883
}
883-
if (restriction_is_constant_false(restrictlist, false)&&
884+
if (restriction_is_constant_false(restrictlist,joinrel,false)&&
884885
bms_is_subset(rel2->relids,sjinfo->syn_righthand))
885886
mark_dummy_rel(rel2);
886887
add_paths_to_joinrel(root,joinrel,rel1,rel2,
@@ -1227,18 +1228,21 @@ mark_dummy_rel(RelOptInfo *rel)
12271228

12281229

12291230
/*
1230-
* restriction_is_constant_false --- is a restrictlist justFALSE?
1231+
* restriction_is_constant_false --- is a restrictlist justfalse?
12311232
*
1232-
* In cases where a qual is provably constantFALSE, eval_const_expressions
1233+
* In cases where a qual is provably constantfalse, eval_const_expressions
12331234
* will generally have thrown away anything that's ANDed with it. In outer
12341235
* join situations this will leave us computing cartesian products only to
12351236
* decide there's no match for an outer row, which is pretty stupid. So,
12361237
* we need to detect the case.
12371238
*
1238-
* If only_pushed_down is TRUE, then consider only pushed-down quals.
1239+
* If only_pushed_down is true, then consider only quals that are pushed-down
1240+
* from the point of view of the joinrel.
12391241
*/
12401242
staticbool
1241-
restriction_is_constant_false(List*restrictlist,boolonly_pushed_down)
1243+
restriction_is_constant_false(List*restrictlist,
1244+
RelOptInfo*joinrel,
1245+
boolonly_pushed_down)
12421246
{
12431247
ListCell*lc;
12441248

@@ -1252,7 +1256,7 @@ restriction_is_constant_false(List *restrictlist, bool only_pushed_down)
12521256
{
12531257
RestrictInfo*rinfo=lfirst_node(RestrictInfo,lc);
12541258

1255-
if (only_pushed_down&& !rinfo->is_pushed_down)
1259+
if (only_pushed_down&& !RINFO_IS_PUSHED_DOWN(rinfo,joinrel->relids))
12561260
continue;
12571261

12581262
if (rinfo->clause&&IsA(rinfo->clause,Const))

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -253,8 +253,7 @@ join_is_removable(PlannerInfo *root, SpecialJoinInfo *sjinfo)
253253
* above the outer join, even if it references no other rels (it might
254254
* be from WHERE, for example).
255255
*/
256-
if (restrictinfo->is_pushed_down||
257-
!bms_equal(restrictinfo->required_relids,joinrelids))
256+
if (RINFO_IS_PUSHED_DOWN(restrictinfo,joinrelids))
258257
{
259258
/*
260259
* If such a clause actually references the inner rel then join
@@ -422,8 +421,7 @@ remove_rel_from_query(PlannerInfo *root, int relid, Relids joinrelids)
422421

423422
remove_join_clause_from_rels(root,rinfo,rinfo->required_relids);
424423

425-
if (rinfo->is_pushed_down||
426-
!bms_equal(rinfo->required_relids,joinrelids))
424+
if (RINFO_IS_PUSHED_DOWN(rinfo,joinrelids))
427425
{
428426
/* Recheck that qual doesn't actually reference the target rel */
429427
Assert(!bms_is_member(relid,rinfo->clause_relids));
@@ -1080,6 +1078,7 @@ is_innerrel_unique_for(PlannerInfo *root,
10801078
JoinTypejointype,
10811079
List*restrictlist)
10821080
{
1081+
Relidsjoinrelids=bms_union(outerrelids,innerrel->relids);
10831082
List*clause_list=NIL;
10841083
ListCell*lc;
10851084

@@ -1098,7 +1097,8 @@ is_innerrel_unique_for(PlannerInfo *root,
10981097
* As noted above, if it's a pushed-down clause and we're at an outer
10991098
* join, we can't use it.
11001099
*/
1101-
if (restrictinfo->is_pushed_down&&IS_OUTER_JOIN(jointype))
1100+
if (IS_OUTER_JOIN(jointype)&&
1101+
RINFO_IS_PUSHED_DOWN(restrictinfo,joinrelids))
11021102
continue;
11031103

11041104
/* Ignore if it's not a mergejoinable clause */

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1754,6 +1754,11 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
17541754
* attach quals to the lowest level where they can be evaluated. But
17551755
* if we were ever to re-introduce a mechanism for delaying evaluation
17561756
* of "expensive" quals, this area would need work.
1757+
*
1758+
* Note: generally, use of is_pushed_down has to go through the macro
1759+
* RINFO_IS_PUSHED_DOWN, because that flag alone is not always sufficient
1760+
* to tell whether a clause must be treated as pushed-down in context.
1761+
* This seems like another reason why it should perhaps be rethought.
17571762
*----------
17581763
*/
17591764
if (is_deduced)

‎src/backend/optimizer/util/restrictinfo.c

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ extract_actual_clauses(List *restrictinfo_list,
371371
* extract_actual_join_clauses
372372
*
373373
* Extract bare clauses from 'restrictinfo_list', separating those that
374-
*syntactically match the join level from those that were pushed down.
374+
*semantically match the join level from those that were pushed down.
375375
* Pseudoconstant clauses are excluded from the results.
376376
*
377377
* This is only used at outer joins, since for plain joins we don't care
@@ -392,15 +392,7 @@ extract_actual_join_clauses(List *restrictinfo_list,
392392
{
393393
RestrictInfo*rinfo=lfirst_node(RestrictInfo,l);
394394

395-
/*
396-
* We must check both is_pushed_down and required_relids, since an
397-
* outer-join clause that's been pushed down to some lower join level
398-
* via path parameterization will not be marked is_pushed_down;
399-
* nonetheless, it must be treated as a filter clause not a join
400-
* clause so far as the lower join level is concerned.
401-
*/
402-
if (rinfo->is_pushed_down||
403-
!bms_is_subset(rinfo->required_relids,joinrelids))
395+
if (RINFO_IS_PUSHED_DOWN(rinfo,joinrelids))
404396
{
405397
if (!rinfo->pseudoconstant)
406398
*otherquals=lappend(*otherquals,rinfo->clause);

‎src/include/nodes/relation.h

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1669,7 +1669,8 @@ typedef struct LimitPath
16691669
* if we decide that it can be pushed down into the nullable side of the join.
16701670
* In that case it acts as a plain filter qual for wherever it gets evaluated.
16711671
* (In short, is_pushed_down is only false for non-degenerate outer join
1672-
* conditions. Possibly we should rename it to reflect that meaning?)
1672+
* conditions. Possibly we should rename it to reflect that meaning? But
1673+
* see also the comments for RINFO_IS_PUSHED_DOWN, below.)
16731674
*
16741675
* RestrictInfo nodes also contain an outerjoin_delayed flag, which is true
16751676
* if the clause's applicability must be delayed due to any outer joins
@@ -1809,6 +1810,20 @@ typedef struct RestrictInfo
18091810
Selectivityright_bucketsize;/* avg bucketsize of right side */
18101811
}RestrictInfo;
18111812

1813+
/*
1814+
* This macro embodies the correct way to test whether a RestrictInfo is
1815+
* "pushed down" to a given outer join, that is, should be treated as a filter
1816+
* clause rather than a join clause at that outer join. This is certainly so
1817+
* if is_pushed_down is true; but examining that is not sufficient anymore,
1818+
* because outer-join clauses will get pushed down to lower outer joins when
1819+
* we generate a path for the lower outer join that is parameterized by the
1820+
* LHS of the upper one. We can detect such a clause by noting that its
1821+
* required_relids exceed the scope of the join.
1822+
*/
1823+
#defineRINFO_IS_PUSHED_DOWN(rinfo,joinrelids) \
1824+
((rinfo)->is_pushed_down || \
1825+
!bms_is_subset((rinfo)->required_relids, joinrelids))
1826+
18121827
/*
18131828
* Since mergejoinscansel() is a relatively expensive function, and would
18141829
* otherwise be invoked many times while planning a large join tree,

0 commit comments

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