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

Commit64ad858

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 parent306d6e5 commit64ad858

File tree

7 files changed

+52
-32
lines changed

7 files changed

+52
-32
lines changed

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ static bool has_indexed_join_quals(NestPath *joinpath);
147147
staticdoubleapprox_tuple_count(PlannerInfo*root,JoinPath*path,
148148
List*quals);
149149
staticdoublecalc_joinrel_size_estimate(PlannerInfo*root,
150+
RelOptInfo*joinrel,
150151
RelOptInfo*outer_rel,
151152
RelOptInfo*inner_rel,
152153
doubleouter_rows,
@@ -3542,13 +3543,15 @@ compute_semi_anti_join_factors(PlannerInfo *root,
35423543
*/
35433544
if (jointype==JOIN_ANTI)
35443545
{
3546+
Relidsjoinrelids=bms_union(outerrel->relids,innerrel->relids);
3547+
35453548
joinquals=NIL;
35463549
foreach(l,restrictlist)
35473550
{
35483551
RestrictInfo*rinfo= (RestrictInfo*)lfirst(l);
35493552

35503553
Assert(IsA(rinfo,RestrictInfo));
3551-
if (!rinfo->is_pushed_down)
3554+
if (!RINFO_IS_PUSHED_DOWN(rinfo,joinrelids))
35523555
joinquals=lappend(joinquals,rinfo);
35533556
}
35543557
}
@@ -3863,6 +3866,7 @@ set_joinrel_size_estimates(PlannerInfo *root, RelOptInfo *rel,
38633866
List*restrictlist)
38643867
{
38653868
rel->rows=calc_joinrel_size_estimate(root,
3869+
rel,
38663870
outer_rel,
38673871
inner_rel,
38683872
outer_rel->rows,
@@ -3905,6 +3909,7 @@ get_parameterized_joinrel_size(PlannerInfo *root, RelOptInfo *rel,
39053909
* estimate for any pair with the same parameterization.
39063910
*/
39073911
nrows=calc_joinrel_size_estimate(root,
3912+
rel,
39083913
outer_path->parent,
39093914
inner_path->parent,
39103915
outer_path->rows,
@@ -3928,6 +3933,7 @@ get_parameterized_joinrel_size(PlannerInfo *root, RelOptInfo *rel,
39283933
*/
39293934
staticdouble
39303935
calc_joinrel_size_estimate(PlannerInfo*root,
3936+
RelOptInfo*joinrel,
39313937
RelOptInfo*outer_rel,
39323938
RelOptInfo*inner_rel,
39333939
doubleouter_rows,
@@ -3981,7 +3987,7 @@ calc_joinrel_size_estimate(PlannerInfo *root,
39813987
RestrictInfo*rinfo= (RestrictInfo*)lfirst(l);
39823988

39833989
Assert(IsA(rinfo,RestrictInfo));
3984-
if (rinfo->is_pushed_down)
3990+
if (RINFO_IS_PUSHED_DOWN(rinfo,joinrel->relids))
39853991
pushedquals=lappend(pushedquals,rinfo);
39863992
else
39873993
joinquals=lappend(joinquals,rinfo);

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

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

13141314
if (!restrictinfo->can_join||
@@ -1547,7 +1547,7 @@ select_mergejoin_clauses(PlannerInfo *root,
15471547
* we don't set have_nonmergeable_joinclause here because pushed-down
15481548
* clauses will become otherquals not joinquals.)
15491549
*/
1550-
if (isouterjoin&&restrictinfo->is_pushed_down)
1550+
if (isouterjoin&&RINFO_IS_PUSHED_DOWN(restrictinfo,joinrel->relids))
15511551
continue;
15521552

15531553
/* 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

3637

@@ -746,7 +747,7 @@ make_join_rel(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2)
746747
{
747748
caseJOIN_INNER:
748749
if (is_dummy_rel(rel1)||is_dummy_rel(rel2)||
749-
restriction_is_constant_false(restrictlist, false))
750+
restriction_is_constant_false(restrictlist,joinrel,false))
750751
{
751752
mark_dummy_rel(joinrel);
752753
break;
@@ -760,12 +761,12 @@ make_join_rel(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2)
760761
break;
761762
caseJOIN_LEFT:
762763
if (is_dummy_rel(rel1)||
763-
restriction_is_constant_false(restrictlist, true))
764+
restriction_is_constant_false(restrictlist,joinrel,true))
764765
{
765766
mark_dummy_rel(joinrel);
766767
break;
767768
}
768-
if (restriction_is_constant_false(restrictlist, false)&&
769+
if (restriction_is_constant_false(restrictlist,joinrel,false)&&
769770
bms_is_subset(rel2->relids,sjinfo->syn_righthand))
770771
mark_dummy_rel(rel2);
771772
add_paths_to_joinrel(root,joinrel,rel1,rel2,
@@ -777,7 +778,7 @@ make_join_rel(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2)
777778
break;
778779
caseJOIN_FULL:
779780
if ((is_dummy_rel(rel1)&&is_dummy_rel(rel2))||
780-
restriction_is_constant_false(restrictlist, true))
781+
restriction_is_constant_false(restrictlist,joinrel,true))
781782
{
782783
mark_dummy_rel(joinrel);
783784
break;
@@ -813,7 +814,7 @@ make_join_rel(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2)
813814
bms_is_subset(sjinfo->min_righthand,rel2->relids))
814815
{
815816
if (is_dummy_rel(rel1)||is_dummy_rel(rel2)||
816-
restriction_is_constant_false(restrictlist, false))
817+
restriction_is_constant_false(restrictlist,joinrel,false))
817818
{
818819
mark_dummy_rel(joinrel);
819820
break;
@@ -836,7 +837,7 @@ make_join_rel(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2)
836837
sjinfo)!=NULL)
837838
{
838839
if (is_dummy_rel(rel1)||is_dummy_rel(rel2)||
839-
restriction_is_constant_false(restrictlist, false))
840+
restriction_is_constant_false(restrictlist,joinrel,false))
840841
{
841842
mark_dummy_rel(joinrel);
842843
break;
@@ -851,12 +852,12 @@ make_join_rel(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2)
851852
break;
852853
caseJOIN_ANTI:
853854
if (is_dummy_rel(rel1)||
854-
restriction_is_constant_false(restrictlist, true))
855+
restriction_is_constant_false(restrictlist,joinrel,true))
855856
{
856857
mark_dummy_rel(joinrel);
857858
break;
858859
}
859-
if (restriction_is_constant_false(restrictlist, false)&&
860+
if (restriction_is_constant_false(restrictlist,joinrel,false)&&
860861
bms_is_subset(rel2->relids,sjinfo->syn_righthand))
861862
mark_dummy_rel(rel2);
862863
add_paths_to_joinrel(root,joinrel,rel1,rel2,
@@ -1207,18 +1208,21 @@ mark_dummy_rel(RelOptInfo *rel)
12071208

12081209

12091210
/*
1210-
* restriction_is_constant_false --- is a restrictlist justFALSE?
1211+
* restriction_is_constant_false --- is a restrictlist justfalse?
12111212
*
1212-
* In cases where a qual is provably constantFALSE, eval_const_expressions
1213+
* In cases where a qual is provably constantfalse, eval_const_expressions
12131214
* will generally have thrown away anything that's ANDed with it. In outer
12141215
* join situations this will leave us computing cartesian products only to
12151216
* decide there's no match for an outer row, which is pretty stupid. So,
12161217
* we need to detect the case.
12171218
*
1218-
* If only_pushed_down is TRUE, then consider only pushed-down quals.
1219+
* If only_pushed_down is true, then consider only quals that are pushed-down
1220+
* from the point of view of the joinrel.
12191221
*/
12201222
staticbool
1221-
restriction_is_constant_false(List*restrictlist,boolonly_pushed_down)
1223+
restriction_is_constant_false(List*restrictlist,
1224+
RelOptInfo*joinrel,
1225+
boolonly_pushed_down)
12221226
{
12231227
ListCell*lc;
12241228

@@ -1233,7 +1237,7 @@ restriction_is_constant_false(List *restrictlist, bool only_pushed_down)
12331237
RestrictInfo*rinfo= (RestrictInfo*)lfirst(lc);
12341238

12351239
Assert(IsA(rinfo,RestrictInfo));
1236-
if (only_pushed_down&& !rinfo->is_pushed_down)
1240+
if (only_pushed_down&& !RINFO_IS_PUSHED_DOWN(rinfo,joinrel->relids))
12371241
continue;
12381242

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

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -248,8 +248,7 @@ join_is_removable(PlannerInfo *root, SpecialJoinInfo *sjinfo)
248248
* above the outer join, even if it references no other rels (it might
249249
* be from WHERE, for example).
250250
*/
251-
if (restrictinfo->is_pushed_down||
252-
!bms_equal(restrictinfo->required_relids,joinrelids))
251+
if (RINFO_IS_PUSHED_DOWN(restrictinfo,joinrelids))
253252
{
254253
/*
255254
* If such a clause actually references the inner rel then join
@@ -417,8 +416,7 @@ remove_rel_from_query(PlannerInfo *root, int relid, Relids joinrelids)
417416

418417
remove_join_clause_from_rels(root,rinfo,rinfo->required_relids);
419418

420-
if (rinfo->is_pushed_down||
421-
!bms_equal(rinfo->required_relids,joinrelids))
419+
if (RINFO_IS_PUSHED_DOWN(rinfo,joinrelids))
422420
{
423421
/* Recheck that qual doesn't actually reference the target rel */
424422
Assert(!bms_is_member(relid,rinfo->clause_relids));

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

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

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

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -410,7 +410,7 @@ extract_actual_clauses(List *restrictinfo_list,
410410
* extract_actual_join_clauses
411411
*
412412
* Extract bare clauses from 'restrictinfo_list', separating those that
413-
*syntactically match the join level from those that were pushed down.
413+
*semantically match the join level from those that were pushed down.
414414
* Pseudoconstant clauses are excluded from the results.
415415
*
416416
* This is only used at outer joins, since for plain joins we don't care
@@ -433,15 +433,7 @@ extract_actual_join_clauses(List *restrictinfo_list,
433433

434434
Assert(IsA(rinfo,RestrictInfo));
435435

436-
/*
437-
* We must check both is_pushed_down and required_relids, since an
438-
* outer-join clause that's been pushed down to some lower join level
439-
* via path parameterization will not be marked is_pushed_down;
440-
* nonetheless, it must be treated as a filter clause not a join
441-
* clause so far as the lower join level is concerned.
442-
*/
443-
if (rinfo->is_pushed_down||
444-
!bms_is_subset(rinfo->required_relids,joinrelids))
436+
if (RINFO_IS_PUSHED_DOWN(rinfo,joinrelids))
445437
{
446438
if (!rinfo->pseudoconstant)
447439
*otherquals=lappend(*otherquals,rinfo->clause);

‎src/include/nodes/relation.h

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1537,7 +1537,8 @@ typedef struct LimitPath
15371537
* if we decide that it can be pushed down into the nullable side of the join.
15381538
* In that case it acts as a plain filter qual for wherever it gets evaluated.
15391539
* (In short, is_pushed_down is only false for non-degenerate outer join
1540-
* conditions. Possibly we should rename it to reflect that meaning?)
1540+
* conditions. Possibly we should rename it to reflect that meaning? But
1541+
* see also the comments for RINFO_IS_PUSHED_DOWN, below.)
15411542
*
15421543
* RestrictInfo nodes also contain an outerjoin_delayed flag, which is true
15431544
* if the clause's applicability must be delayed due to any outer joins
@@ -1664,6 +1665,20 @@ typedef struct RestrictInfo
16641665
Selectivityright_bucketsize;/* avg bucketsize of right side */
16651666
}RestrictInfo;
16661667

1668+
/*
1669+
* This macro embodies the correct way to test whether a RestrictInfo is
1670+
* "pushed down" to a given outer join, that is, should be treated as a filter
1671+
* clause rather than a join clause at that outer join. This is certainly so
1672+
* if is_pushed_down is true; but examining that is not sufficient anymore,
1673+
* because outer-join clauses will get pushed down to lower outer joins when
1674+
* we generate a path for the lower outer join that is parameterized by the
1675+
* LHS of the upper one. We can detect such a clause by noting that its
1676+
* required_relids exceed the scope of the join.
1677+
*/
1678+
#defineRINFO_IS_PUSHED_DOWN(rinfo,joinrelids) \
1679+
((rinfo)->is_pushed_down || \
1680+
!bms_is_subset((rinfo)->required_relids, joinrelids))
1681+
16671682
/*
16681683
* Since mergejoinscansel() is a relatively expensive function, and would
16691684
* otherwise be invoked many times while planning a large join tree,

0 commit comments

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