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

Commit8a7f24b

Browse files
committed
Repair bogus EPQ plans generated for postgres_fdw foreign joins.
postgres_fdw's postgresGetForeignPlan() assumes without checking that theouter_plan it's given for a join relation must have a NestLoop, MergeJoin,or HashJoin node at the top. That's been wrong at least since commit4bbf6ed (which could cause insertion of a Sort node on top) and it seemslike a pretty unsafe thing to Just Assume even without that.Through blind good fortune, this doesn't seem to have any worseconsequences today than strange EXPLAIN output, but it's clearly troublewaiting to happen.To fix, test the node type explicitly before touching Join-specificfields, and avoid jamming the new tlist into a node type that can'tdo projection. Export a new support function from createplan.cto avoid building low-level knowledge about the latter into FDWs.Back-patch to 9.6 where the faulty coding was added. Note that theassociated regression test cases don't show any changes before v11,apparently because the tests back-patched with4bbf6ed don't actuallyexercise the problem case before then (there's no top-level Sortin those plans).Discussion:https://postgr.es/m/8946.1544644803@sss.pgh.pa.us
1 parentf9f63ed commit8a7f24b

File tree

3 files changed

+68
-25
lines changed

3 files changed

+68
-25
lines changed

‎contrib/postgres_fdw/postgres_fdw.c

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1195,11 +1195,9 @@ postgresGetForeignPlan(PlannerInfo *root,
11951195

11961196
/*
11971197
* Ensure that the outer plan produces a tuple whose descriptor
1198-
* matches our scan tuple slot. This is safe because all scans and
1199-
* joins support projection, so we never need to insert a Result node.
1200-
* Also, remove the local conditions from outer plan's quals, lest
1201-
* they will be evaluated twice, once by the local plan and once by
1202-
* the scan.
1198+
* matches our scan tuple slot. Also, remove the local conditions
1199+
* from outer plan's quals, lest they be evaluated twice, once by the
1200+
* local plan and once by the scan.
12031201
*/
12041202
if (outer_plan)
12051203
{
@@ -1212,23 +1210,42 @@ postgresGetForeignPlan(PlannerInfo *root,
12121210
*/
12131211
Assert(!IS_UPPER_REL(foreignrel));
12141212

1215-
outer_plan->targetlist=fdw_scan_tlist;
1216-
1213+
/*
1214+
* First, update the plan's qual list if possible. In some cases
1215+
* the quals might be enforced below the topmost plan level, in
1216+
* which case we'll fail to remove them; it's not worth working
1217+
* harder than this.
1218+
*/
12171219
foreach(lc,local_exprs)
12181220
{
1219-
Join*join_plan= (Join*)outer_plan;
12201221
Node*qual=lfirst(lc);
12211222

12221223
outer_plan->qual=list_delete(outer_plan->qual,qual);
12231224

12241225
/*
12251226
* For an inner join the local conditions of foreign scan plan
1226-
* can be part of the joinquals as well.
1227+
* can be part of the joinquals as well. (They might also be
1228+
* in the mergequals or hashquals, but we can't touch those
1229+
* without breaking the plan.)
12271230
*/
1228-
if (join_plan->jointype==JOIN_INNER)
1229-
join_plan->joinqual=list_delete(join_plan->joinqual,
1230-
qual);
1231+
if (IsA(outer_plan,NestLoop)||
1232+
IsA(outer_plan,MergeJoin)||
1233+
IsA(outer_plan,HashJoin))
1234+
{
1235+
Join*join_plan= (Join*)outer_plan;
1236+
1237+
if (join_plan->jointype==JOIN_INNER)
1238+
join_plan->joinqual=list_delete(join_plan->joinqual,
1239+
qual);
1240+
}
12311241
}
1242+
1243+
/*
1244+
* Now fix the subplan's tlist --- this might result in inserting
1245+
* a Result node atop the plan tree.
1246+
*/
1247+
outer_plan=change_plan_targetlist(outer_plan,fdw_scan_tlist,
1248+
best_path->path.parallel_safe);
12321249
}
12331250
}
12341251

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

Lines changed: 37 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1317,20 +1317,10 @@ create_unique_plan(PlannerInfo *root, UniquePath *best_path, int flags)
13171317
}
13181318
}
13191319

1320+
/* Use change_plan_targetlist in case we need to insert a Result node */
13201321
if (newitems||best_path->umethod==UNIQUE_PATH_SORT)
1321-
{
1322-
/*
1323-
* If the top plan node can't do projections and its existing target
1324-
* list isn't already what we need, we need to add a Result node to
1325-
* help it along.
1326-
*/
1327-
if (!is_projection_capable_plan(subplan)&&
1328-
!tlist_same_exprs(newtlist,subplan->targetlist))
1329-
subplan=inject_projection_plan(subplan,newtlist,
1330-
best_path->path.parallel_safe);
1331-
else
1332-
subplan->targetlist=newtlist;
1333-
}
1322+
subplan=change_plan_targetlist(subplan,newtlist,
1323+
best_path->path.parallel_safe);
13341324

13351325
/*
13361326
* Build control information showing which subplan output columns are to
@@ -1635,6 +1625,40 @@ inject_projection_plan(Plan *subplan, List *tlist, bool parallel_safe)
16351625
returnplan;
16361626
}
16371627

1628+
/*
1629+
* change_plan_targetlist
1630+
* Externally available wrapper for inject_projection_plan.
1631+
*
1632+
* This is meant for use by FDW plan-generation functions, which might
1633+
* want to adjust the tlist computed by some subplan tree. In general,
1634+
* a Result node is needed to compute the new tlist, but we can optimize
1635+
* some cases.
1636+
*
1637+
* In most cases, tlist_parallel_safe can just be passed as the parallel_safe
1638+
* flag of the FDW's own Path node.
1639+
*/
1640+
Plan*
1641+
change_plan_targetlist(Plan*subplan,List*tlist,booltlist_parallel_safe)
1642+
{
1643+
/*
1644+
* If the top plan node can't do projections and its existing target list
1645+
* isn't already what we need, we need to add a Result node to help it
1646+
* along.
1647+
*/
1648+
if (!is_projection_capable_plan(subplan)&&
1649+
!tlist_same_exprs(tlist,subplan->targetlist))
1650+
subplan=inject_projection_plan(subplan,tlist,
1651+
subplan->parallel_safe&&
1652+
tlist_parallel_safe);
1653+
else
1654+
{
1655+
/* Else we can just replace the plan node's tlist */
1656+
subplan->targetlist=tlist;
1657+
subplan->parallel_safe &=tlist_parallel_safe;
1658+
}
1659+
returnsubplan;
1660+
}
1661+
16381662
/*
16391663
* create_sort_plan
16401664
*

‎src/include/optimizer/planmain.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ extern ForeignScan *make_foreignscan(List *qptlist, List *qpqual,
5252
Indexscanrelid,List*fdw_exprs,List*fdw_private,
5353
List*fdw_scan_tlist,List*fdw_recheck_quals,
5454
Plan*outer_plan);
55+
externPlan*change_plan_targetlist(Plan*subplan,List*tlist,
56+
booltlist_parallel_safe);
5557
externPlan*materialize_finished_plan(Plan*subplan);
5658
externboolis_projection_capable_path(Path*path);
5759
externboolis_projection_capable_plan(Plan*plan);

0 commit comments

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