aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--contrib/file_fdw/file_fdw.c6
-rw-r--r--contrib/postgres_fdw/expected/postgres_fdw.out56
-rw-r--r--contrib/postgres_fdw/postgres_fdw.c90
-rw-r--r--contrib/postgres_fdw/sql/postgres_fdw.sql26
-rw-r--r--doc/src/sgml/fdwhandler.sgml8
-rw-r--r--src/backend/optimizer/util/pathnode.c111
-rw-r--r--src/backend/optimizer/util/relnode.c9
-rw-r--r--src/include/optimizer/pathnode.h13
8 files changed, 275 insertions, 44 deletions
diff --git a/contrib/file_fdw/file_fdw.c b/contrib/file_fdw/file_fdw.c
index be626bef6e6..391cd0d778d 100644
--- a/contrib/file_fdw/file_fdw.c
+++ b/contrib/file_fdw/file_fdw.c
@@ -556,6 +556,10 @@ fileGetForeignPaths(PlannerInfo *root,
* Create a ForeignPath node and add it as only possible path. We use the
* fdw_private list of the path to carry the convert_selectively option;
* it will be propagated into the fdw_private list of the Plan node.
+ *
+ * We don't support pushing join clauses into the quals of this path, but
+ * it could still have required parameterization due to LATERAL refs in
+ * its tlist.
*/
add_path(baserel, (Path *)
create_foreignscan_path(root, baserel,
@@ -564,7 +568,7 @@ fileGetForeignPaths(PlannerInfo *root,
startup_cost,
total_cost,
NIL, /* no pathkeys */
- NULL, /* no outer rel either */
+ baserel->lateral_relids,
NULL, /* no extra plan */
coptions));
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index b3894d07607..029552b152b 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -3429,6 +3429,62 @@ select c2, sum from "S 1"."T 1" t1, lateral (select sum(t2.c1 + t1."C 1") sum fr
(2 rows)
reset enable_hashagg;
+-- bug #15613: bad plan for foreign table scan with lateral reference
+EXPLAIN (VERBOSE, COSTS OFF)
+SELECT ref_0.c2, subq_1.*
+FROM
+ "S 1"."T 1" AS ref_0,
+ LATERAL (
+ SELECT ref_0."C 1" c1, subq_0.*
+ FROM (SELECT ref_0.c2, ref_1.c3
+ FROM ft1 AS ref_1) AS subq_0
+ RIGHT JOIN ft2 AS ref_3 ON (subq_0.c3 = ref_3.c3)
+ ) AS subq_1
+WHERE ref_0."C 1" < 10 AND subq_1.c3 = '00001'
+ORDER BY ref_0."C 1";
+ QUERY PLAN
+---------------------------------------------------------------------------------------------------------
+ Nested Loop
+ Output: ref_0.c2, ref_0."C 1", (ref_0.c2), ref_1.c3, ref_0."C 1"
+ -> Nested Loop
+ Output: ref_0.c2, ref_0."C 1", ref_1.c3, (ref_0.c2)
+ -> Index Scan using t1_pkey on "S 1"."T 1" ref_0
+ Output: ref_0."C 1", ref_0.c2, ref_0.c3, ref_0.c4, ref_0.c5, ref_0.c6, ref_0.c7, ref_0.c8
+ Index Cond: (ref_0."C 1" < 10)
+ -> Foreign Scan on public.ft1 ref_1
+ Output: ref_1.c3, ref_0.c2
+ Remote SQL: SELECT c3 FROM "S 1"."T 1" WHERE ((c3 = '00001'::text))
+ -> Materialize
+ Output: ref_3.c3
+ -> Foreign Scan on public.ft2 ref_3
+ Output: ref_3.c3
+ Remote SQL: SELECT c3 FROM "S 1"."T 1" WHERE ((c3 = '00001'::text))
+(15 rows)
+
+SELECT ref_0.c2, subq_1.*
+FROM
+ "S 1"."T 1" AS ref_0,
+ LATERAL (
+ SELECT ref_0."C 1" c1, subq_0.*
+ FROM (SELECT ref_0.c2, ref_1.c3
+ FROM ft1 AS ref_1) AS subq_0
+ RIGHT JOIN ft2 AS ref_3 ON (subq_0.c3 = ref_3.c3)
+ ) AS subq_1
+WHERE ref_0."C 1" < 10 AND subq_1.c3 = '00001'
+ORDER BY ref_0."C 1";
+ c2 | c1 | c2 | c3
+----+----+----+-------
+ 1 | 1 | 1 | 00001
+ 2 | 2 | 2 | 00001
+ 3 | 3 | 3 | 00001
+ 4 | 4 | 4 | 00001
+ 5 | 5 | 5 | 00001
+ 6 | 6 | 6 | 00001
+ 7 | 7 | 7 | 00001
+ 8 | 8 | 8 | 00001
+ 9 | 9 | 9 | 00001
+(9 rows)
+
-- Check with placeHolderVars
explain (verbose, costs off)
select sum(q.a), count(q.b) from ft4 left join (select 13, avg(ft1.c1), sum(ft2.c1) from ft1 right join ft2 on (ft1.c1 = ft2.c1)) q(a, b, c) on (ft4.c1 <= q.b);
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);
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index f438165650c..bab8479cf6a 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -879,6 +879,32 @@ select c2, sum from "S 1"."T 1" t1, lateral (select sum(t2.c1 + t1."C 1") sum fr
select c2, sum from "S 1"."T 1" t1, lateral (select sum(t2.c1 + t1."C 1") sum from ft2 t2 group by t2.c1) qry where t1.c2 * 2 = qry.sum and t1.c2 < 3 and t1."C 1" < 100 order by 1;
reset enable_hashagg;
+-- bug #15613: bad plan for foreign table scan with lateral reference
+EXPLAIN (VERBOSE, COSTS OFF)
+SELECT ref_0.c2, subq_1.*
+FROM
+ "S 1"."T 1" AS ref_0,
+ LATERAL (
+ SELECT ref_0."C 1" c1, subq_0.*
+ FROM (SELECT ref_0.c2, ref_1.c3
+ FROM ft1 AS ref_1) AS subq_0
+ RIGHT JOIN ft2 AS ref_3 ON (subq_0.c3 = ref_3.c3)
+ ) AS subq_1
+WHERE ref_0."C 1" < 10 AND subq_1.c3 = '00001'
+ORDER BY ref_0."C 1";
+
+SELECT ref_0.c2, subq_1.*
+FROM
+ "S 1"."T 1" AS ref_0,
+ LATERAL (
+ SELECT ref_0."C 1" c1, subq_0.*
+ FROM (SELECT ref_0.c2, ref_1.c3
+ FROM ft1 AS ref_1) AS subq_0
+ RIGHT JOIN ft2 AS ref_3 ON (subq_0.c3 = ref_3.c3)
+ ) AS subq_1
+WHERE ref_0."C 1" < 10 AND subq_1.c3 = '00001'
+ORDER BY ref_0."C 1";
+
-- Check with placeHolderVars
explain (verbose, costs off)
select sum(q.a), count(q.b) from ft4 left join (select 13, avg(ft1.c1), sum(ft2.c1) from ft1 right join ft2 on (ft1.c1 = ft2.c1)) q(a, b, c) on (ft4.c1 <= q.b);
diff --git a/doc/src/sgml/fdwhandler.sgml b/doc/src/sgml/fdwhandler.sgml
index 452b776b9e1..77038f53f9a 100644
--- a/doc/src/sgml/fdwhandler.sgml
+++ b/doc/src/sgml/fdwhandler.sgml
@@ -309,7 +309,9 @@ GetForeignJoinPaths(PlannerInfo *root,
function is called during query planning. As
with <function>GetForeignPaths</function>, this function should
generate <structname>ForeignPath</structname> path(s) for the
- supplied <literal>joinrel</literal>, and call <function>add_path</function> to add these
+ supplied <literal>joinrel</literal>
+ (use <function>create_foreign_join_path</function> to build them),
+ and call <function>add_path</function> to add these
paths to the set of paths considered for the join. But unlike
<function>GetForeignPaths</function>, it is not necessary that this function
succeed in creating at least one path, since paths involving local
@@ -369,7 +371,9 @@ GetForeignUpperPaths(PlannerInfo *root,
called only if all base relation(s) involved in the query belong to the
same FDW. This function should generate <structname>ForeignPath</structname>
path(s) for any post-scan/join processing that the FDW knows how to
- perform remotely, and call <function>add_path</function> to add these paths to
+ perform remotely
+ (use <function>create_foreign_upper_path</function> to build them),
+ and call <function>add_path</function> to add these paths to
the indicated upper relation. As with <function>GetForeignJoinPaths</function>,
it is not necessary that this function succeed in creating any paths,
since paths involving local processing are always possible.
diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index b57de6b4c67..08133a28fd2 100644
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -2079,15 +2079,14 @@ create_worktablescan_path(PlannerInfo *root, RelOptInfo *rel,
/*
* create_foreignscan_path
- * Creates a path corresponding to a scan of a foreign table, foreign join,
- * or foreign upper-relation processing, returning the pathnode.
+ * Creates a path corresponding to a scan of a foreign base table,
+ * returning the pathnode.
*
* This function is never called from core Postgres; rather, it's expected
- * to be called by the GetForeignPaths, GetForeignJoinPaths, or
- * GetForeignUpperPaths function of a foreign data wrapper. We make the FDW
- * supply all fields of the path, since we do not have any way to calculate
- * them in core. However, there is a usually-sane default for the pathtarget
- * (rel->reltarget), so we let a NULL for "target" select that.
+ * to be called by the GetForeignPaths function of a foreign data wrapper.
+ * We make the FDW supply all fields of the path, since we do not have any way
+ * to calculate them in core. However, there is a usually-sane default for
+ * the pathtarget (rel->reltarget), so we let a NULL for "target" select that.
*/
ForeignPath *
create_foreignscan_path(PlannerInfo *root, RelOptInfo *rel,
@@ -2100,6 +2099,9 @@ create_foreignscan_path(PlannerInfo *root, RelOptInfo *rel,
{
ForeignPath *pathnode = makeNode(ForeignPath);
+ /* Historically some FDWs were confused about when to use this */
+ Assert(IS_SIMPLE_REL(rel));
+
pathnode->path.pathtype = T_ForeignScan;
pathnode->path.parent = rel;
pathnode->path.pathtarget = target ? target : rel->reltarget;
@@ -2120,6 +2122,101 @@ create_foreignscan_path(PlannerInfo *root, RelOptInfo *rel,
}
/*
+ * create_foreign_join_path
+ * Creates a path corresponding to a scan of a foreign join,
+ * returning the pathnode.
+ *
+ * This function is never called from core Postgres; rather, it's expected
+ * to be called by the GetForeignJoinPaths function of a foreign data wrapper.
+ * We make the FDW supply all fields of the path, since we do not have any way
+ * to calculate them in core. However, there is a usually-sane default for
+ * the pathtarget (rel->reltarget), so we let a NULL for "target" select that.
+ */
+ForeignPath *
+create_foreign_join_path(PlannerInfo *root, RelOptInfo *rel,
+ PathTarget *target,
+ double rows, Cost startup_cost, Cost total_cost,
+ List *pathkeys,
+ Relids required_outer,
+ Path *fdw_outerpath,
+ List *fdw_private)
+{
+ ForeignPath *pathnode = makeNode(ForeignPath);
+
+ /*
+ * We should use get_joinrel_parampathinfo to handle parameterized paths,
+ * but the API of this function doesn't support it, and existing
+ * extensions aren't yet trying to build such paths anyway. For the
+ * moment just throw an error if someone tries it; eventually we should
+ * revisit this.
+ */
+ if (!bms_is_empty(required_outer) || !bms_is_empty(rel->lateral_relids))
+ elog(ERROR, "parameterized foreign joins are not supported yet");
+
+ pathnode->path.pathtype = T_ForeignScan;
+ pathnode->path.parent = rel;
+ pathnode->path.pathtarget = target ? target : rel->reltarget;
+ pathnode->path.param_info = NULL; /* XXX see above */
+ pathnode->path.parallel_aware = false;
+ pathnode->path.parallel_safe = rel->consider_parallel;
+ pathnode->path.parallel_workers = 0;
+ pathnode->path.rows = rows;
+ pathnode->path.startup_cost = startup_cost;
+ pathnode->path.total_cost = total_cost;
+ pathnode->path.pathkeys = pathkeys;
+
+ pathnode->fdw_outerpath = fdw_outerpath;
+ pathnode->fdw_private = fdw_private;
+
+ return pathnode;
+}
+
+/*
+ * create_foreign_upper_path
+ * Creates a path corresponding to an upper relation that's computed
+ * directly by an FDW, returning the pathnode.
+ *
+ * This function is never called from core Postgres; rather, it's expected to
+ * be called by the GetForeignUpperPaths function of a foreign data wrapper.
+ * We make the FDW supply all fields of the path, since we do not have any way
+ * to calculate them in core. However, there is a usually-sane default for
+ * the pathtarget (rel->reltarget), so we let a NULL for "target" select that.
+ */
+ForeignPath *
+create_foreign_upper_path(PlannerInfo *root, RelOptInfo *rel,
+ PathTarget *target,
+ double rows, Cost startup_cost, Cost total_cost,
+ List *pathkeys,
+ Path *fdw_outerpath,
+ List *fdw_private)
+{
+ ForeignPath *pathnode = makeNode(ForeignPath);
+
+ /*
+ * Upper relations should never have any lateral references, since joining
+ * is complete.
+ */
+ Assert(bms_is_empty(rel->lateral_relids));
+
+ pathnode->path.pathtype = T_ForeignScan;
+ pathnode->path.parent = rel;
+ pathnode->path.pathtarget = target ? target : rel->reltarget;
+ pathnode->path.param_info = NULL;
+ pathnode->path.parallel_aware = false;
+ pathnode->path.parallel_safe = rel->consider_parallel;
+ pathnode->path.parallel_workers = 0;
+ pathnode->path.rows = rows;
+ pathnode->path.startup_cost = startup_cost;
+ pathnode->path.total_cost = total_cost;
+ pathnode->path.pathkeys = pathkeys;
+
+ pathnode->fdw_outerpath = fdw_outerpath;
+ pathnode->fdw_private = fdw_private;
+
+ return pathnode;
+}
+
+/*
* calc_nestloop_required_outer
* Compute the required_outer set for a nestloop join path
*
diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c
index f04c6b76f49..4130514952b 100644
--- a/src/backend/optimizer/util/relnode.c
+++ b/src/backend/optimizer/util/relnode.c
@@ -1225,6 +1225,9 @@ get_baserel_parampathinfo(PlannerInfo *root, RelOptInfo *baserel,
double rows;
ListCell *lc;
+ /* If rel has LATERAL refs, every path for it should account for them */
+ Assert(bms_is_subset(baserel->lateral_relids, required_outer));
+
/* Unparameterized paths have no ParamPathInfo */
if (bms_is_empty(required_outer))
return NULL;
@@ -1320,6 +1323,9 @@ get_joinrel_parampathinfo(PlannerInfo *root, RelOptInfo *joinrel,
double rows;
ListCell *lc;
+ /* If rel has LATERAL refs, every path for it should account for them */
+ Assert(bms_is_subset(joinrel->lateral_relids, required_outer));
+
/* Unparameterized paths have no ParamPathInfo or extra join clauses */
if (bms_is_empty(required_outer))
return NULL;
@@ -1511,6 +1517,9 @@ get_appendrel_parampathinfo(RelOptInfo *appendrel, Relids required_outer)
{
ParamPathInfo *ppi;
+ /* If rel has LATERAL refs, every path for it should account for them */
+ Assert(bms_is_subset(appendrel->lateral_relids, required_outer));
+
/* Unparameterized paths have no ParamPathInfo */
if (bms_is_empty(required_outer))
return NULL;
diff --git a/src/include/optimizer/pathnode.h b/src/include/optimizer/pathnode.h
index d0c8f99d0a5..ef2c9b47289 100644
--- a/src/include/optimizer/pathnode.h
+++ b/src/include/optimizer/pathnode.h
@@ -118,6 +118,19 @@ extern ForeignPath *create_foreignscan_path(PlannerInfo *root, RelOptInfo *rel,
Relids required_outer,
Path *fdw_outerpath,
List *fdw_private);
+extern ForeignPath *create_foreign_join_path(PlannerInfo *root, RelOptInfo *rel,
+ PathTarget *target,
+ double rows, Cost startup_cost, Cost total_cost,
+ List *pathkeys,
+ Relids required_outer,
+ Path *fdw_outerpath,
+ List *fdw_private);
+extern ForeignPath *create_foreign_upper_path(PlannerInfo *root, RelOptInfo *rel,
+ PathTarget *target,
+ double rows, Cost startup_cost, Cost total_cost,
+ List *pathkeys,
+ Path *fdw_outerpath,
+ List *fdw_private);
extern Relids calc_nestloop_required_outer(Relids outerrelids,
Relids outer_paramrels,