aboutsummaryrefslogtreecommitdiff
path: root/contrib/postgres_fdw/postgres_fdw.c
diff options
context:
space:
mode:
authorTom Lane <tgl@sss.pgh.pa.us>2019-02-07 12:59:47 -0500
committerTom Lane <tgl@sss.pgh.pa.us>2019-02-07 13:11:12 -0500
commit34ea1ab7fd305afe1124a6e73ada0ebae04b6ebb (patch)
tree28134be91fb909a218fa661b2bcd3fa6c3e04475 /contrib/postgres_fdw/postgres_fdw.c
parent51b025933d442823b076e36f4dbe756d25b1a159 (diff)
downloadpostgresql-34ea1ab7fd305afe1124a6e73ada0ebae04b6ebb.tar.gz
postgresql-34ea1ab7fd305afe1124a6e73ada0ebae04b6ebb.zip
Split create_foreignscan_path() into three functions.
Up to now postgres_fdw has been using create_foreignscan_path() to generate not only base-relation paths, but also paths for foreign joins and foreign upperrels. This is wrong, because create_foreignscan_path() calls get_baserel_parampathinfo() which will only do the right thing for baserels. It accidentally fails to fail for unparameterized paths, which are the only ones postgres_fdw (thought it) was handling, but we really need different APIs for the baserel and join cases. In HEAD, the best thing to do seems to be to split up the baserel, joinrel, and upperrel cases into three functions so that they can have different APIs. I haven't actually given create_foreign_join_path a different API in this commit: we should spend a bit of time thinking about just what we want to do there, since perhaps FDWs would want to do something different from the build-up-a-join-pairwise approach that get_joinrel_parampathinfo expects. In the meantime, since postgres_fdw isn't prepared to generate parameterized joins anyway, just give it a defense against trying to plan joins with lateral refs. In addition (and this is what triggered this whole mess) fix bug #15613 from Srinivasan S A, by teaching file_fdw and postgres_fdw that plain baserel foreign paths still have outer refs if the relation has lateral_relids. Add some assertions in relnode.c to catch future occurrences of the same error --- in particular, to catch other FDWs doing that, but also as backstop against core-code mistakes like the one fixed by commit bdd9a99aa. Bug #15613 also needs to be fixed in the back branches, but the appropriate fix will look quite a bit different there, since we don't want to assume that existing FDWs get the word right away. Discussion: https://postgr.es/m/15613-092be1be9576c728@postgresql.org
Diffstat (limited to 'contrib/postgres_fdw/postgres_fdw.c')
-rw-r--r--contrib/postgres_fdw/postgres_fdw.c90
1 files changed, 56 insertions, 34 deletions
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 7fcac81e2e4..994cec50ce8 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -937,6 +937,9 @@ postgresGetForeignPaths(PlannerInfo *root,
* baserestrict conditions we were able to send to remote, there might
* actually be an indexscan happening there). We already did all the work
* to estimate cost and size of this path.
+ *
+ * Although this path uses no join clauses, it could still have required
+ * parameterization due to LATERAL refs in its tlist.
*/
path = create_foreignscan_path(root, baserel,
NULL, /* default pathtarget */
@@ -944,7 +947,7 @@ postgresGetForeignPaths(PlannerInfo *root,
fpinfo->startup_cost,
fpinfo->total_cost,
NIL, /* no pathkeys */
- NULL, /* no outer rel either */
+ baserel->lateral_relids,
NULL, /* no extra plan */
NIL); /* no fdw_private list */
add_path(baserel, (Path *) path);
@@ -3295,7 +3298,7 @@ execute_foreign_modify(EState *estate,
TupleTableSlot *planSlot)
{
PgFdwModifyState *fmstate = (PgFdwModifyState *) resultRelInfo->ri_FdwState;
- ItemPointer ctid = NULL;
+ ItemPointer ctid = NULL;
const char **p_values;
PGresult *res;
int n_rows;
@@ -3936,8 +3939,9 @@ apply_returning_filter(PgFdwDirectModifyState *dmstate,
ExecStoreVirtualTuple(resultSlot);
/*
- * If we have any system columns to return, materialize a heap tuple in the
- * slot from column values set above and install system columns in that tuple.
+ * If we have any system columns to return, materialize a heap tuple in
+ * the slot from column values set above and install system columns in
+ * that tuple.
*/
if (dmstate->hasSystemCols)
{
@@ -4943,16 +4947,28 @@ add_paths_with_pathkeys_for_rel(PlannerInfo *root, RelOptInfo *rel,
useful_pathkeys,
-1.0);
- add_path(rel, (Path *)
- create_foreignscan_path(root, rel,
- NULL,
- rows,
- startup_cost,
- total_cost,
- useful_pathkeys,
- NULL,
- sorted_epq_path,
- NIL));
+ if (IS_SIMPLE_REL(rel))
+ add_path(rel, (Path *)
+ create_foreignscan_path(root, rel,
+ NULL,
+ rows,
+ startup_cost,
+ total_cost,
+ useful_pathkeys,
+ rel->lateral_relids,
+ sorted_epq_path,
+ NIL));
+ else
+ add_path(rel, (Path *)
+ create_foreign_join_path(root, rel,
+ NULL,
+ rows,
+ startup_cost,
+ total_cost,
+ useful_pathkeys,
+ rel->lateral_relids,
+ sorted_epq_path,
+ NIL));
}
}
@@ -5088,6 +5104,13 @@ postgresGetForeignJoinPaths(PlannerInfo *root,
return;
/*
+ * This code does not work for joins with lateral references, since those
+ * must have parameterized paths, which we don't generate yet.
+ */
+ if (!bms_is_empty(joinrel->lateral_relids))
+ return;
+
+ /*
* Create unfinished PgFdwRelationInfo entry which is used to indicate
* that the join relation is already considered, so that we won't waste
* time in judging safety of join pushdown and adding the same paths again
@@ -5171,16 +5194,16 @@ postgresGetForeignJoinPaths(PlannerInfo *root,
* Create a new join path and add it to the joinrel which represents a
* join between foreign tables.
*/
- joinpath = create_foreignscan_path(root,
- joinrel,
- NULL, /* default pathtarget */
- rows,
- startup_cost,
- total_cost,
- NIL, /* no pathkeys */
- NULL, /* no required_outer */
- epq_path,
- NIL); /* no fdw_private */
+ joinpath = create_foreign_join_path(root,
+ joinrel,
+ NULL, /* default pathtarget */
+ rows,
+ startup_cost,
+ total_cost,
+ NIL, /* no pathkeys */
+ joinrel->lateral_relids,
+ epq_path,
+ NIL); /* no fdw_private */
/* Add generated path into joinrel by add_path(). */
add_path(joinrel, (Path *) joinpath);
@@ -5515,16 +5538,15 @@ add_foreign_grouping_paths(PlannerInfo *root, RelOptInfo *input_rel,
fpinfo->total_cost = total_cost;
/* Create and add foreign path to the grouping relation. */
- grouppath = create_foreignscan_path(root,
- grouped_rel,
- grouped_rel->reltarget,
- rows,
- startup_cost,
- total_cost,
- NIL, /* no pathkeys */
- NULL, /* no required_outer */
- NULL,
- NIL); /* no fdw_private */
+ grouppath = create_foreign_upper_path(root,
+ grouped_rel,
+ grouped_rel->reltarget,
+ rows,
+ startup_cost,
+ total_cost,
+ NIL, /* no pathkeys */
+ NULL,
+ NIL); /* no fdw_private */
/* Add generated path into grouped_rel by add_path(). */
add_path(grouped_rel, (Path *) grouppath);