database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-06-11 20:28:21 +03:00
Code Activity

Repair bogus EPQ plans generated for postgres_fdw foreign joins.

Browse Source 
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 commit
4bbf6edfb (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 with 4bbf6edfb 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:
Tom Lane
2018-12-12 16:08:30 -05:00
parent 0f7ec8d9c3
commit 77d4d88afb
4 changed files with 140 additions and 89 deletions
Download Patch File Download Diff File Expand all files Collapse all files

41
contrib/postgres_fdw/postgres_fdw.c
View File

@ -1229,11 +1229,9 @@ postgresGetForeignPlan(PlannerInfo *root,
/*
* Ensure that the outer plan produces a tuple whose descriptor
* matches our scan tuple slot. This is safe because all scans and
* joins support projection, so we never need to insert a Result node.
* Also, remove the local conditions from outer plan's quals, lest
* they will be evaluated twice, once by the local plan and once by
* the scan.
* matches our scan tuple slot. Also, remove the local conditions
* from outer plan's quals, lest they be evaluated twice, once by the
* local plan and once by the scan.
*/
if (outer_plan)
{
@ -1246,23 +1244,42 @@ postgresGetForeignPlan(PlannerInfo *root,
*/
Assert(!IS_UPPER_REL(foreignrel));
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)
{
Join *join_plan = (Join *) outer_plan;
Node *qual = lfirst(lc);
outer_plan->qual = list_delete(outer_plan->qual, qual);
/*
* 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)
join_plan->joinqual = list_delete(join_plan->joinqual,
qual);
if (IsA(outer_plan, NestLoop) ||
IsA(outer_plan, MergeJoin) ||
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);
}
}