mirror of
https://git.postgresql.org/git/postgresql.git
synced 2024-08-27 21:47:18 +02:00
Repair bogus EPQ plans generated for postgres_fdw foreign joins.
postgres_fdw's postgresGetForeignPlan() assumes without checking that the outer_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 commit4bbf6edfb
(which could cause insertion of a Sort node on top) and it seems like a pretty unsafe thing to Just Assume even without that. Through blind good fortune, this doesn't seem to have any worse consequences today than strange EXPLAIN output, but it's clearly trouble waiting to happen. To fix, test the node type explicitly before touching Join-specific fields, and avoid jamming the new tlist into a node type that can't do projection. Export a new support function from createplan.c to avoid building low-level knowledge about the latter into FDWs. Back-patch to 9.6 where the faulty coding was added. Note that the associated regression test cases don't show any changes before v11, apparently because the tests back-patched with4bbf6edfb
don't actually exercise the problem case before then (there's no top-level Sort in those plans). Discussion: https://postgr.es/m/8946.1544644803@sss.pgh.pa.us
This commit is contained in:
parent
10bad8c0f0
commit
12cb7ea599
@ -1181,33 +1181,50 @@ postgresGetForeignPlan(PlannerInfo *root,
|
|||||||
|
|
||||||
/*
|
/*
|
||||||
* Ensure that the outer plan produces a tuple whose descriptor
|
* Ensure that the outer plan produces a tuple whose descriptor
|
||||||
* matches our scan tuple slot. This is safe because all scans and
|
* matches our scan tuple slot. Also, remove the local conditions
|
||||||
* joins support projection, so we never need to insert a Result node.
|
* from outer plan's quals, lest they be evaluated twice, once by the
|
||||||
* Also, remove the local conditions from outer plan's quals, lest
|
* local plan and once by the scan.
|
||||||
* they will be evaluated twice, once by the local plan and once by
|
|
||||||
* the scan.
|
|
||||||
*/
|
*/
|
||||||
if (outer_plan)
|
if (outer_plan)
|
||||||
{
|
{
|
||||||
ListCell *lc;
|
ListCell *lc;
|
||||||
|
|
||||||
outer_plan->targetlist = fdw_scan_tlist;
|
/*
|
||||||
|
* First, update the plan's qual list if possible. In some cases
|
||||||
|
* the quals might be enforced below the topmost plan level, in
|
||||||
|
* which case we'll fail to remove them; it's not worth working
|
||||||
|
* harder than this.
|
||||||
|
*/
|
||||||
foreach(lc, local_exprs)
|
foreach(lc, local_exprs)
|
||||||
{
|
{
|
||||||
Join *join_plan = (Join *) outer_plan;
|
|
||||||
Node *qual = lfirst(lc);
|
Node *qual = lfirst(lc);
|
||||||
|
|
||||||
outer_plan->qual = list_delete(outer_plan->qual, qual);
|
outer_plan->qual = list_delete(outer_plan->qual, qual);
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* For an inner join the local conditions of foreign scan plan
|
* For an inner join the local conditions of foreign scan plan
|
||||||
* can be part of the joinquals as well.
|
* can be part of the joinquals as well. (They might also be
|
||||||
|
* in the mergequals or hashquals, but we can't touch those
|
||||||
|
* without breaking the plan.)
|
||||||
*/
|
*/
|
||||||
if (join_plan->jointype == JOIN_INNER)
|
if (IsA(outer_plan, NestLoop) ||
|
||||||
join_plan->joinqual = list_delete(join_plan->joinqual,
|
IsA(outer_plan, MergeJoin) ||
|
||||||
qual);
|
IsA(outer_plan, HashJoin))
|
||||||
|
{
|
||||||
|
Join *join_plan = (Join *) outer_plan;
|
||||||
|
|
||||||
|
if (join_plan->jointype == JOIN_INNER)
|
||||||
|
join_plan->joinqual = list_delete(join_plan->joinqual,
|
||||||
|
qual);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/*
|
||||||
|
* Now fix the subplan's tlist --- this might result in inserting
|
||||||
|
* a Result node atop the plan tree.
|
||||||
|
*/
|
||||||
|
outer_plan = change_plan_targetlist(outer_plan, fdw_scan_tlist,
|
||||||
|
best_path->path.parallel_safe);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -1252,19 +1252,10 @@ create_unique_plan(PlannerInfo *root, UniquePath *best_path, int flags)
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* Use change_plan_targetlist in case we need to insert a Result node */
|
||||||
if (newitems || best_path->umethod == UNIQUE_PATH_SORT)
|
if (newitems || best_path->umethod == UNIQUE_PATH_SORT)
|
||||||
{
|
subplan = change_plan_targetlist(subplan, newtlist,
|
||||||
/*
|
best_path->path.parallel_safe);
|
||||||
* If the top plan node can't do projections and its existing target
|
|
||||||
* list isn't already what we need, we need to add a Result node to
|
|
||||||
* help it along.
|
|
||||||
*/
|
|
||||||
if (!is_projection_capable_plan(subplan) &&
|
|
||||||
!tlist_same_exprs(newtlist, subplan->targetlist))
|
|
||||||
subplan = inject_projection_plan(subplan, newtlist);
|
|
||||||
else
|
|
||||||
subplan->targetlist = newtlist;
|
|
||||||
}
|
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Build control information showing which subplan output columns are to
|
* Build control information showing which subplan output columns are to
|
||||||
@ -1505,6 +1496,37 @@ inject_projection_plan(Plan *subplan, List *tlist)
|
|||||||
return plan;
|
return plan;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/*
|
||||||
|
* change_plan_targetlist
|
||||||
|
* Externally available wrapper for inject_projection_plan.
|
||||||
|
*
|
||||||
|
* This is meant for use by FDW plan-generation functions, which might
|
||||||
|
* want to adjust the tlist computed by some subplan tree. In general,
|
||||||
|
* a Result node is needed to compute the new tlist, but we can optimize
|
||||||
|
* some cases.
|
||||||
|
*
|
||||||
|
* In most cases, tlist_parallel_safe can just be passed as the parallel_safe
|
||||||
|
* flag of the FDW's own Path node. (It's not actually used in this branch.)
|
||||||
|
*/
|
||||||
|
Plan *
|
||||||
|
change_plan_targetlist(Plan *subplan, List *tlist, bool tlist_parallel_safe)
|
||||||
|
{
|
||||||
|
/*
|
||||||
|
* If the top plan node can't do projections and its existing target list
|
||||||
|
* isn't already what we need, we need to add a Result node to help it
|
||||||
|
* along.
|
||||||
|
*/
|
||||||
|
if (!is_projection_capable_plan(subplan) &&
|
||||||
|
!tlist_same_exprs(tlist, subplan->targetlist))
|
||||||
|
subplan = inject_projection_plan(subplan, tlist);
|
||||||
|
else
|
||||||
|
{
|
||||||
|
/* Else we can just replace the plan node's tlist */
|
||||||
|
subplan->targetlist = tlist;
|
||||||
|
}
|
||||||
|
return subplan;
|
||||||
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* create_sort_plan
|
* create_sort_plan
|
||||||
*
|
*
|
||||||
|
@ -52,6 +52,8 @@ extern ForeignScan *make_foreignscan(List *qptlist, List *qpqual,
|
|||||||
Index scanrelid, List *fdw_exprs, List *fdw_private,
|
Index scanrelid, List *fdw_exprs, List *fdw_private,
|
||||||
List *fdw_scan_tlist, List *fdw_recheck_quals,
|
List *fdw_scan_tlist, List *fdw_recheck_quals,
|
||||||
Plan *outer_plan);
|
Plan *outer_plan);
|
||||||
|
extern Plan *change_plan_targetlist(Plan *subplan, List *tlist,
|
||||||
|
bool tlist_parallel_safe);
|
||||||
extern Plan *materialize_finished_plan(Plan *subplan);
|
extern Plan *materialize_finished_plan(Plan *subplan);
|
||||||
extern bool is_projection_capable_path(Path *path);
|
extern bool is_projection_capable_path(Path *path);
|
||||||
extern bool is_projection_capable_plan(Plan *plan);
|
extern bool is_projection_capable_plan(Plan *plan);
|
||||||
|
Loading…
Reference in New Issue
Block a user