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

Commitc792c7d

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 parent68c23cb commitc792c7d

File tree

10 files changed

+64
-37
lines changed

10 files changed

+64
-37
lines changed

‎contrib/postgres_fdw/postgres_fdw.c‎

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

4708-
if (IS_OUTER_JOIN(jointype)&& !rinfo->is_pushed_down)
4708+
if (IS_OUTER_JOIN(jointype)&&
4709+
!RINFO_IS_PUSHED_DOWN(rinfo,joinrel->relids))
47094710
{
47104711
if (!is_remote_clause)
47114712
return false;

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,7 @@ static bool has_indexed_join_quals(NestPath *joinpath);
159159
staticdoubleapprox_tuple_count(PlannerInfo*root,JoinPath*path,
160160
List*quals);
161161
staticdoublecalc_joinrel_size_estimate(PlannerInfo*root,
162+
RelOptInfo*joinrel,
162163
RelOptInfo*outer_rel,
163164
RelOptInfo*inner_rel,
164165
doubleouter_rows,
@@ -4055,12 +4056,14 @@ compute_semi_anti_join_factors(PlannerInfo *root,
40554056
*/
40564057
if (IS_OUTER_JOIN(jointype))
40574058
{
4059+
Relidsjoinrelids=bms_union(outerrel->relids,innerrel->relids);
4060+
40584061
joinquals=NIL;
40594062
foreach(l,restrictlist)
40604063
{
40614064
RestrictInfo*rinfo=lfirst_node(RestrictInfo,l);
40624065

4063-
if (!rinfo->is_pushed_down)
4066+
if (!RINFO_IS_PUSHED_DOWN(rinfo,joinrelids))
40644067
joinquals=lappend(joinquals,rinfo);
40654068
}
40664069
}
@@ -4375,6 +4378,7 @@ set_joinrel_size_estimates(PlannerInfo *root, RelOptInfo *rel,
43754378
List*restrictlist)
43764379
{
43774380
rel->rows=calc_joinrel_size_estimate(root,
4381+
rel,
43784382
outer_rel,
43794383
inner_rel,
43804384
outer_rel->rows,
@@ -4417,6 +4421,7 @@ get_parameterized_joinrel_size(PlannerInfo *root, RelOptInfo *rel,
44174421
* estimate for any pair with the same parameterization.
44184422
*/
44194423
nrows=calc_joinrel_size_estimate(root,
4424+
rel,
44204425
outer_path->parent,
44214426
inner_path->parent,
44224427
outer_path->rows,
@@ -4440,6 +4445,7 @@ get_parameterized_joinrel_size(PlannerInfo *root, RelOptInfo *rel,
44404445
*/
44414446
staticdouble
44424447
calc_joinrel_size_estimate(PlannerInfo*root,
4448+
RelOptInfo*joinrel,
44434449
RelOptInfo*outer_rel,
44444450
RelOptInfo*inner_rel,
44454451
doubleouter_rows,
@@ -4492,7 +4498,7 @@ calc_joinrel_size_estimate(PlannerInfo *root,
44924498
{
44934499
RestrictInfo*rinfo=lfirst_node(RestrictInfo,l);
44944500

4495-
if (rinfo->is_pushed_down)
4501+
if (RINFO_IS_PUSHED_DOWN(rinfo,joinrel->relids))
44964502
pushedquals=lappend(pushedquals,rinfo);
44974503
else
44984504
joinquals=lappend(joinquals,rinfo);

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

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

17061706
if (!restrictinfo->can_join||
@@ -1947,7 +1947,7 @@ select_mergejoin_clauses(PlannerInfo *root,
19471947
* we don't set have_nonmergeable_joinclause here because pushed-down
19481948
* clauses will become otherquals not joinquals.)
19491949
*/
1950-
if (isouterjoin&&restrictinfo->is_pushed_down)
1950+
if (isouterjoin&&RINFO_IS_PUSHED_DOWN(restrictinfo,joinrel->relids))
19511951
continue;
19521952

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

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

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ static bool has_join_restriction(PlannerInfo *root, RelOptInfo *rel);
3535
staticboolhas_legal_joinclause(PlannerInfo*root,RelOptInfo*rel);
3636
staticboolis_dummy_rel(RelOptInfo*rel);
3737
staticboolrestriction_is_constant_false(List*restrictlist,
38+
RelOptInfo*joinrel,
3839
boolonly_pushed_down);
3940
staticvoidpopulate_joinrel_with_paths(PlannerInfo*root,RelOptInfo*rel1,
4041
RelOptInfo*rel2,RelOptInfo*joinrel,
@@ -780,7 +781,7 @@ populate_joinrel_with_paths(PlannerInfo *root, RelOptInfo *rel1,
780781
{
781782
caseJOIN_INNER:
782783
if (is_dummy_rel(rel1)||is_dummy_rel(rel2)||
783-
restriction_is_constant_false(restrictlist, false))
784+
restriction_is_constant_false(restrictlist,joinrel,false))
784785
{
785786
mark_dummy_rel(joinrel);
786787
break;
@@ -794,12 +795,12 @@ populate_joinrel_with_paths(PlannerInfo *root, RelOptInfo *rel1,
794795
break;
795796
caseJOIN_LEFT:
796797
if (is_dummy_rel(rel1)||
797-
restriction_is_constant_false(restrictlist, true))
798+
restriction_is_constant_false(restrictlist,joinrel,true))
798799
{
799800
mark_dummy_rel(joinrel);
800801
break;
801802
}
802-
if (restriction_is_constant_false(restrictlist, false)&&
803+
if (restriction_is_constant_false(restrictlist,joinrel,false)&&
803804
bms_is_subset(rel2->relids,sjinfo->syn_righthand))
804805
mark_dummy_rel(rel2);
805806
add_paths_to_joinrel(root,joinrel,rel1,rel2,
@@ -811,7 +812,7 @@ populate_joinrel_with_paths(PlannerInfo *root, RelOptInfo *rel1,
811812
break;
812813
caseJOIN_FULL:
813814
if ((is_dummy_rel(rel1)&&is_dummy_rel(rel2))||
814-
restriction_is_constant_false(restrictlist, true))
815+
restriction_is_constant_false(restrictlist,joinrel,true))
815816
{
816817
mark_dummy_rel(joinrel);
817818
break;
@@ -847,7 +848,7 @@ populate_joinrel_with_paths(PlannerInfo *root, RelOptInfo *rel1,
847848
bms_is_subset(sjinfo->min_righthand,rel2->relids))
848849
{
849850
if (is_dummy_rel(rel1)||is_dummy_rel(rel2)||
850-
restriction_is_constant_false(restrictlist, false))
851+
restriction_is_constant_false(restrictlist,joinrel,false))
851852
{
852853
mark_dummy_rel(joinrel);
853854
break;
@@ -870,7 +871,7 @@ populate_joinrel_with_paths(PlannerInfo *root, RelOptInfo *rel1,
870871
sjinfo)!=NULL)
871872
{
872873
if (is_dummy_rel(rel1)||is_dummy_rel(rel2)||
873-
restriction_is_constant_false(restrictlist, false))
874+
restriction_is_constant_false(restrictlist,joinrel,false))
874875
{
875876
mark_dummy_rel(joinrel);
876877
break;
@@ -885,12 +886,12 @@ populate_joinrel_with_paths(PlannerInfo *root, RelOptInfo *rel1,
885886
break;
886887
caseJOIN_ANTI:
887888
if (is_dummy_rel(rel1)||
888-
restriction_is_constant_false(restrictlist, true))
889+
restriction_is_constant_false(restrictlist,joinrel,true))
889890
{
890891
mark_dummy_rel(joinrel);
891892
break;
892893
}
893-
if (restriction_is_constant_false(restrictlist, false)&&
894+
if (restriction_is_constant_false(restrictlist,joinrel,false)&&
894895
bms_is_subset(rel2->relids,sjinfo->syn_righthand))
895896
mark_dummy_rel(rel2);
896897
add_paths_to_joinrel(root,joinrel,rel1,rel2,
@@ -1249,10 +1250,13 @@ mark_dummy_rel(RelOptInfo *rel)
12491250
* decide there's no match for an outer row, which is pretty stupid. So,
12501251
* we need to detect the case.
12511252
*
1252-
* If only_pushed_down is true, then consider only pushed-down quals.
1253+
* If only_pushed_down is true, then consider only quals that are pushed-down
1254+
* from the point of view of the joinrel.
12531255
*/
12541256
staticbool
1255-
restriction_is_constant_false(List*restrictlist,boolonly_pushed_down)
1257+
restriction_is_constant_false(List*restrictlist,
1258+
RelOptInfo*joinrel,
1259+
boolonly_pushed_down)
12561260
{
12571261
ListCell*lc;
12581262

@@ -1266,7 +1270,7 @@ restriction_is_constant_false(List *restrictlist, bool only_pushed_down)
12661270
{
12671271
RestrictInfo*rinfo=lfirst_node(RestrictInfo,lc);
12681272

1269-
if (only_pushed_down&& !rinfo->is_pushed_down)
1273+
if (only_pushed_down&& !RINFO_IS_PUSHED_DOWN(rinfo,joinrel->relids))
12701274
continue;
12711275

12721276
if (rinfo->clause&&IsA(rinfo->clause,Const))
@@ -1411,8 +1415,9 @@ try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,
14111415
* partition keys from given relations being joined.
14121416
*/
14131417
bool
1414-
have_partkey_equi_join(RelOptInfo*rel1,RelOptInfo*rel2,JoinTypejointype,
1415-
List*restrictlist)
1418+
have_partkey_equi_join(RelOptInfo*joinrel,
1419+
RelOptInfo*rel1,RelOptInfo*rel2,
1420+
JoinTypejointype,List*restrictlist)
14161421
{
14171422
PartitionSchemepart_scheme=rel1->part_scheme;
14181423
ListCell*lc;
@@ -1438,7 +1443,8 @@ have_partkey_equi_join(RelOptInfo *rel1, RelOptInfo *rel2, JoinType jointype,
14381443
intipk2;
14391444

14401445
/* If processing an outer join, only use its own join clauses. */
1441-
if (IS_OUTER_JOIN(jointype)&&rinfo->is_pushed_down)
1446+
if (IS_OUTER_JOIN(jointype)&&
1447+
RINFO_IS_PUSHED_DOWN(rinfo,joinrel->relids))
14421448
continue;
14431449

14441450
/* Skip clauses which can not be used for a join. */

‎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
@@ -1775,6 +1775,11 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
17751775
* attach quals to the lowest level where they can be evaluated. But
17761776
* if we were ever to re-introduce a mechanism for delaying evaluation
17771777
* of "expensive" quals, this area would need work.
1778+
*
1779+
* Note: generally, use of is_pushed_down has to go through the macro
1780+
* RINFO_IS_PUSHED_DOWN, because that flag alone is not always sufficient
1781+
* to tell whether a clause must be treated as pushed-down in context.
1782+
* This seems like another reason why it should perhaps be rethought.
17781783
*----------
17791784
*/
17801785
if (is_deduced)

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1629,7 +1629,8 @@ build_joinrel_partition_info(RelOptInfo *joinrel, RelOptInfo *outer_rel,
16291629
*/
16301630
if (!IS_PARTITIONED_REL(outer_rel)|| !IS_PARTITIONED_REL(inner_rel)||
16311631
outer_rel->part_scheme!=inner_rel->part_scheme||
1632-
!have_partkey_equi_join(outer_rel,inner_rel,jointype,restrictlist))
1632+
!have_partkey_equi_join(joinrel,outer_rel,inner_rel,
1633+
jointype,restrictlist))
16331634
{
16341635
Assert(!IS_PARTITIONED_REL(joinrel));
16351636
return;

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

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

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

‎src/include/nodes/relation.h‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1789,7 +1789,8 @@ typedef struct LimitPath
17891789
* if we decide that it can be pushed down into the nullable side of the join.
17901790
* In that case it acts as a plain filter qual for wherever it gets evaluated.
17911791
* (In short, is_pushed_down is only false for non-degenerate outer join
1792-
* conditions. Possibly we should rename it to reflect that meaning?)
1792+
* conditions. Possibly we should rename it to reflect that meaning? But
1793+
* see also the comments for RINFO_IS_PUSHED_DOWN, below.)
17931794
*
17941795
* RestrictInfo nodes also contain an outerjoin_delayed flag, which is true
17951796
* if the clause's applicability must be delayed due to any outer joins
@@ -1931,6 +1932,20 @@ typedef struct RestrictInfo
19311932
Selectivityright_mcvfreq;/* right side's most common val's freq */
19321933
}RestrictInfo;
19331934

1935+
/*
1936+
* This macro embodies the correct way to test whether a RestrictInfo is
1937+
* "pushed down" to a given outer join, that is, should be treated as a filter
1938+
* clause rather than a join clause at that outer join. This is certainly so
1939+
* if is_pushed_down is true; but examining that is not sufficient anymore,
1940+
* because outer-join clauses will get pushed down to lower outer joins when
1941+
* we generate a path for the lower outer join that is parameterized by the
1942+
* LHS of the upper one. We can detect such a clause by noting that its
1943+
* required_relids exceed the scope of the join.
1944+
*/
1945+
#defineRINFO_IS_PUSHED_DOWN(rinfo,joinrelids) \
1946+
((rinfo)->is_pushed_down || \
1947+
!bms_is_subset((rinfo)->required_relids, joinrelids))
1948+
19341949
/*
19351950
* Since mergejoinscansel() is a relatively expensive function, and would
19361951
* otherwise be invoked many times while planning a large join tree,

‎src/include/optimizer/paths.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,8 @@ extern bool have_join_order_restriction(PlannerInfo *root,
115115
externboolhave_dangerous_phv(PlannerInfo*root,
116116
Relidsouter_relids,Relidsinner_params);
117117
externvoidmark_dummy_rel(RelOptInfo*rel);
118-
externboolhave_partkey_equi_join(RelOptInfo*rel1,RelOptInfo*rel2,
118+
externboolhave_partkey_equi_join(RelOptInfo*joinrel,
119+
RelOptInfo*rel1,RelOptInfo*rel2,
119120
JoinTypejointype,List*restrictlist);
120121

121122
/*

0 commit comments

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