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

Commit34ea1ab

Browse files
committed
Split create_foreignscan_path() into three functions.
Up to now postgres_fdw has been using create_foreignscan_path() togenerate not only base-relation paths, but also paths for foreign joinsand foreign upperrels. This is wrong, because create_foreignscan_path()calls get_baserel_parampathinfo() which will only do the right thing forbaserels. It accidentally fails to fail for unparameterized paths, whichare the only ones postgres_fdw (thought it) was handling, but we reallyneed 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 canhave different APIs. I haven't actually given create_foreign_join_patha different API in this commit: we should spend a bit of time thinkingabout just what we want to do there, since perhaps FDWs would want todo something different from the build-up-a-join-pairwise approach thatget_joinrel_parampathinfo expects. In the meantime, since postgres_fdwisn't prepared to generate parameterized joins anyway, just give it adefense against trying to plan joins with lateral refs.In addition (and this is what triggered this whole mess) fix bug #15613from Srinivasan S A, by teaching file_fdw and postgres_fdw that plainbaserel foreign paths still have outer refs if the relation haslateral_relids. Add some assertions in relnode.c to catch futureoccurrences of the same error --- in particular, to catch other FDWsdoing that, but also as backstop against core-code mistakes like theone fixed by commitbdd9a99.Bug #15613 also needs to be fixed in the back branches, but theappropriate fix will look quite a bit different there, since we don'twant to assume that existing FDWs get the word right away.Discussion:https://postgr.es/m/15613-092be1be9576c728@postgresql.org
1 parent51b0259 commit34ea1ab

File tree

8 files changed

+275
-44
lines changed

8 files changed

+275
-44
lines changed

‎contrib/file_fdw/file_fdw.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -556,6 +556,10 @@ fileGetForeignPaths(PlannerInfo *root,
556556
* Create a ForeignPath node and add it as only possible path. We use the
557557
* fdw_private list of the path to carry the convert_selectively option;
558558
* it will be propagated into the fdw_private list of the Plan node.
559+
*
560+
* We don't support pushing join clauses into the quals of this path, but
561+
* it could still have required parameterization due to LATERAL refs in
562+
* its tlist.
559563
*/
560564
add_path(baserel, (Path*)
561565
create_foreignscan_path(root,baserel,
@@ -564,7 +568,7 @@ fileGetForeignPaths(PlannerInfo *root,
564568
startup_cost,
565569
total_cost,
566570
NIL,/* no pathkeys */
567-
NULL,/* no outer rel either */
571+
baserel->lateral_relids,
568572
NULL,/* no extra plan */
569573
coptions));
570574

‎contrib/postgres_fdw/expected/postgres_fdw.out

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3429,6 +3429,62 @@ select c2, sum from "S 1"."T 1" t1, lateral (select sum(t2.c1 + t1."C 1") sum fr
34293429
(2 rows)
34303430

34313431
reset enable_hashagg;
3432+
-- bug #15613: bad plan for foreign table scan with lateral reference
3433+
EXPLAIN (VERBOSE, COSTS OFF)
3434+
SELECT ref_0.c2, subq_1.*
3435+
FROM
3436+
"S 1"."T 1" AS ref_0,
3437+
LATERAL (
3438+
SELECT ref_0."C 1" c1, subq_0.*
3439+
FROM (SELECT ref_0.c2, ref_1.c3
3440+
FROM ft1 AS ref_1) AS subq_0
3441+
RIGHT JOIN ft2 AS ref_3 ON (subq_0.c3 = ref_3.c3)
3442+
) AS subq_1
3443+
WHERE ref_0."C 1" < 10 AND subq_1.c3 = '00001'
3444+
ORDER BY ref_0."C 1";
3445+
QUERY PLAN
3446+
---------------------------------------------------------------------------------------------------------
3447+
Nested Loop
3448+
Output: ref_0.c2, ref_0."C 1", (ref_0.c2), ref_1.c3, ref_0."C 1"
3449+
-> Nested Loop
3450+
Output: ref_0.c2, ref_0."C 1", ref_1.c3, (ref_0.c2)
3451+
-> Index Scan using t1_pkey on "S 1"."T 1" ref_0
3452+
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
3453+
Index Cond: (ref_0."C 1" < 10)
3454+
-> Foreign Scan on public.ft1 ref_1
3455+
Output: ref_1.c3, ref_0.c2
3456+
Remote SQL: SELECT c3 FROM "S 1"."T 1" WHERE ((c3 = '00001'::text))
3457+
-> Materialize
3458+
Output: ref_3.c3
3459+
-> Foreign Scan on public.ft2 ref_3
3460+
Output: ref_3.c3
3461+
Remote SQL: SELECT c3 FROM "S 1"."T 1" WHERE ((c3 = '00001'::text))
3462+
(15 rows)
3463+
3464+
SELECT ref_0.c2, subq_1.*
3465+
FROM
3466+
"S 1"."T 1" AS ref_0,
3467+
LATERAL (
3468+
SELECT ref_0."C 1" c1, subq_0.*
3469+
FROM (SELECT ref_0.c2, ref_1.c3
3470+
FROM ft1 AS ref_1) AS subq_0
3471+
RIGHT JOIN ft2 AS ref_3 ON (subq_0.c3 = ref_3.c3)
3472+
) AS subq_1
3473+
WHERE ref_0."C 1" < 10 AND subq_1.c3 = '00001'
3474+
ORDER BY ref_0."C 1";
3475+
c2 | c1 | c2 | c3
3476+
----+----+----+-------
3477+
1 | 1 | 1 | 00001
3478+
2 | 2 | 2 | 00001
3479+
3 | 3 | 3 | 00001
3480+
4 | 4 | 4 | 00001
3481+
5 | 5 | 5 | 00001
3482+
6 | 6 | 6 | 00001
3483+
7 | 7 | 7 | 00001
3484+
8 | 8 | 8 | 00001
3485+
9 | 9 | 9 | 00001
3486+
(9 rows)
3487+
34323488
-- Check with placeHolderVars
34333489
explain (verbose, costs off)
34343490
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);

‎contrib/postgres_fdw/postgres_fdw.c

Lines changed: 56 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -937,14 +937,17 @@ postgresGetForeignPaths(PlannerInfo *root,
937937
* baserestrict conditions we were able to send to remote, there might
938938
* actually be an indexscan happening there). We already did all the work
939939
* to estimate cost and size of this path.
940+
*
941+
* Although this path uses no join clauses, it could still have required
942+
* parameterization due to LATERAL refs in its tlist.
940943
*/
941944
path=create_foreignscan_path(root,baserel,
942945
NULL,/* default pathtarget */
943946
fpinfo->rows,
944947
fpinfo->startup_cost,
945948
fpinfo->total_cost,
946949
NIL,/* no pathkeys */
947-
NULL,/* no outer rel either */
950+
baserel->lateral_relids,
948951
NULL,/* no extra plan */
949952
NIL);/* no fdw_private list */
950953
add_path(baserel, (Path*)path);
@@ -3295,7 +3298,7 @@ execute_foreign_modify(EState *estate,
32953298
TupleTableSlot*planSlot)
32963299
{
32973300
PgFdwModifyState*fmstate= (PgFdwModifyState*)resultRelInfo->ri_FdwState;
3298-
ItemPointerctid=NULL;
3301+
ItemPointerctid=NULL;
32993302
constchar**p_values;
33003303
PGresult*res;
33013304
intn_rows;
@@ -3936,8 +3939,9 @@ apply_returning_filter(PgFdwDirectModifyState *dmstate,
39363939
ExecStoreVirtualTuple(resultSlot);
39373940

39383941
/*
3939-
* If we have any system columns to return, materialize a heap tuple in the
3940-
* slot from column values set above and install system columns in that tuple.
3942+
* If we have any system columns to return, materialize a heap tuple in
3943+
* the slot from column values set above and install system columns in
3944+
* that tuple.
39413945
*/
39423946
if (dmstate->hasSystemCols)
39433947
{
@@ -4943,16 +4947,28 @@ add_paths_with_pathkeys_for_rel(PlannerInfo *root, RelOptInfo *rel,
49434947
useful_pathkeys,
49444948
-1.0);
49454949

4946-
add_path(rel, (Path*)
4947-
create_foreignscan_path(root,rel,
4948-
NULL,
4949-
rows,
4950-
startup_cost,
4951-
total_cost,
4952-
useful_pathkeys,
4953-
NULL,
4954-
sorted_epq_path,
4955-
NIL));
4950+
if (IS_SIMPLE_REL(rel))
4951+
add_path(rel, (Path*)
4952+
create_foreignscan_path(root,rel,
4953+
NULL,
4954+
rows,
4955+
startup_cost,
4956+
total_cost,
4957+
useful_pathkeys,
4958+
rel->lateral_relids,
4959+
sorted_epq_path,
4960+
NIL));
4961+
else
4962+
add_path(rel, (Path*)
4963+
create_foreign_join_path(root,rel,
4964+
NULL,
4965+
rows,
4966+
startup_cost,
4967+
total_cost,
4968+
useful_pathkeys,
4969+
rel->lateral_relids,
4970+
sorted_epq_path,
4971+
NIL));
49564972
}
49574973
}
49584974

@@ -5087,6 +5103,13 @@ postgresGetForeignJoinPaths(PlannerInfo *root,
50875103
if (joinrel->fdw_private)
50885104
return;
50895105

5106+
/*
5107+
* This code does not work for joins with lateral references, since those
5108+
* must have parameterized paths, which we don't generate yet.
5109+
*/
5110+
if (!bms_is_empty(joinrel->lateral_relids))
5111+
return;
5112+
50905113
/*
50915114
* Create unfinished PgFdwRelationInfo entry which is used to indicate
50925115
* that the join relation is already considered, so that we won't waste
@@ -5171,16 +5194,16 @@ postgresGetForeignJoinPaths(PlannerInfo *root,
51715194
* Create a new join path and add it to the joinrel which represents a
51725195
* join between foreign tables.
51735196
*/
5174-
joinpath=create_foreignscan_path(root,
5175-
joinrel,
5176-
NULL,/* default pathtarget */
5177-
rows,
5178-
startup_cost,
5179-
total_cost,
5180-
NIL,/* no pathkeys */
5181-
NULL,/* no required_outer */
5182-
epq_path,
5183-
NIL);/* no fdw_private */
5197+
joinpath=create_foreign_join_path(root,
5198+
joinrel,
5199+
NULL,/* default pathtarget */
5200+
rows,
5201+
startup_cost,
5202+
total_cost,
5203+
NIL,/* no pathkeys */
5204+
joinrel->lateral_relids,
5205+
epq_path,
5206+
NIL);/* no fdw_private */
51845207

51855208
/* Add generated path into joinrel by add_path(). */
51865209
add_path(joinrel, (Path*)joinpath);
@@ -5515,16 +5538,15 @@ add_foreign_grouping_paths(PlannerInfo *root, RelOptInfo *input_rel,
55155538
fpinfo->total_cost=total_cost;
55165539

55175540
/* Create and add foreign path to the grouping relation. */
5518-
grouppath=create_foreignscan_path(root,
5519-
grouped_rel,
5520-
grouped_rel->reltarget,
5521-
rows,
5522-
startup_cost,
5523-
total_cost,
5524-
NIL,/* no pathkeys */
5525-
NULL,/* no required_outer */
5526-
NULL,
5527-
NIL);/* no fdw_private */
5541+
grouppath=create_foreign_upper_path(root,
5542+
grouped_rel,
5543+
grouped_rel->reltarget,
5544+
rows,
5545+
startup_cost,
5546+
total_cost,
5547+
NIL,/* no pathkeys */
5548+
NULL,
5549+
NIL);/* no fdw_private */
55285550

55295551
/* Add generated path into grouped_rel by add_path(). */
55305552
add_path(grouped_rel, (Path*)grouppath);

‎contrib/postgres_fdw/sql/postgres_fdw.sql

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -879,6 +879,32 @@ select c2, sum from "S 1"."T 1" t1, lateral (select sum(t2.c1 + t1."C 1") sum fr
879879
select c2, sumfrom"S 1"."T 1" t1, lateral (selectsum(t2.c1+ t1."C 1") sumfrom ft2 t2group byt2.c1) qrywheret1.c2*2=qry.sumandt1.c2<3and t1."C 1"<100order by1;
880880
reset enable_hashagg;
881881

882+
-- bug #15613: bad plan for foreign table scan with lateral reference
883+
EXPLAIN (VERBOSE, COSTS OFF)
884+
SELECTref_0.c2, subq_1.*
885+
FROM
886+
"S 1"."T 1"AS ref_0,
887+
LATERAL (
888+
SELECT ref_0."C 1" c1, subq_0.*
889+
FROM (SELECTref_0.c2,ref_1.c3
890+
FROM ft1AS ref_1)AS subq_0
891+
RIGHT JOIN ft2AS ref_3ON (subq_0.c3=ref_3.c3)
892+
)AS subq_1
893+
WHERE ref_0."C 1"<10ANDsubq_1.c3='00001'
894+
ORDER BY ref_0."C 1";
895+
896+
SELECTref_0.c2, subq_1.*
897+
FROM
898+
"S 1"."T 1"AS ref_0,
899+
LATERAL (
900+
SELECT ref_0."C 1" c1, subq_0.*
901+
FROM (SELECTref_0.c2,ref_1.c3
902+
FROM ft1AS ref_1)AS subq_0
903+
RIGHT JOIN ft2AS ref_3ON (subq_0.c3=ref_3.c3)
904+
)AS subq_1
905+
WHERE ref_0."C 1"<10ANDsubq_1.c3='00001'
906+
ORDER BY ref_0."C 1";
907+
882908
-- Check with placeHolderVars
883909
explain (verbose, costs off)
884910
selectsum(q.a),count(q.b)from ft4left join (select13,avg(ft1.c1),sum(ft2.c1)from ft1right join ft2on (ft1.c1=ft2.c1)) q(a, b, c)on (ft4.c1<=q.b);

‎doc/src/sgml/fdwhandler.sgml

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,9 @@ GetForeignJoinPaths(PlannerInfo *root,
309309
function is called during query planning. As
310310
with <function>GetForeignPaths</function>, this function should
311311
generate <structname>ForeignPath</structname> path(s) for the
312-
supplied <literal>joinrel</literal>, and call <function>add_path</function> to add these
312+
supplied <literal>joinrel</literal>
313+
(use <function>create_foreign_join_path</function> to build them),
314+
and call <function>add_path</function> to add these
313315
paths to the set of paths considered for the join. But unlike
314316
<function>GetForeignPaths</function>, it is not necessary that this function
315317
succeed in creating at least one path, since paths involving local
@@ -369,7 +371,9 @@ GetForeignUpperPaths(PlannerInfo *root,
369371
called only if all base relation(s) involved in the query belong to the
370372
same FDW. This function should generate <structname>ForeignPath</structname>
371373
path(s) for any post-scan/join processing that the FDW knows how to
372-
perform remotely, and call <function>add_path</function> to add these paths to
374+
perform remotely
375+
(use <function>create_foreign_upper_path</function> to build them),
376+
and call <function>add_path</function> to add these paths to
373377
the indicated upper relation. As with <function>GetForeignJoinPaths</function>,
374378
it is not necessary that this function succeed in creating any paths,
375379
since paths involving local processing are always possible.

0 commit comments

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