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

Commite3101a0

Browse files
committed
Ensure that foreign scans with lateral refs are planned correctly.
As reported in bug #15613 from Srinivasan S A, file_fdw and postgres_fdwneglected to mark plain baserel foreign paths as parameterized when therelation has lateral_relids. Other FDWs have surely copied this mistake,so rather than just patching those two modules, install a band-aid fixin create_foreignscan_path to rectify the mistake centrally.Although the band-aid is enough to fix the visible symptom, correctthe calls in file_fdw and postgres_fdw anyway, so that they are validexamples for external FDWs.Also, since the band-aid isn't enough to make this work for parameterizedforeign joins, throw an elog(ERROR) if such a case is passed tocreate_foreignscan_path. This shouldn't pose much of a problem forexisting external FDWs, since it's likely they aren't trying to make suchpaths anyway (though some of them may need a defense against joins withlateral_relids, similar to the one this patch installs into postgres_fdw).Add some assertions in relnode.c to catch future occurrences of the sameerror --- in particular, as backstop against core-code mistakes like theone fixed by commitbdd9a99.Discussion:https://postgr.es/m/15613-092be1be9576c728@postgresql.org
1 parentbbcafab commite3101a0

File tree

6 files changed

+131
-5
lines changed

6 files changed

+131
-5
lines changed

‎contrib/file_fdw/file_fdw.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -541,6 +541,10 @@ fileGetForeignPaths(PlannerInfo *root,
541541
* Create a ForeignPath node and add it as only possible path. We use the
542542
* fdw_private list of the path to carry the convert_selectively option;
543543
* it will be propagated into the fdw_private list of the Plan node.
544+
*
545+
* We don't support pushing join clauses into the quals of this path, but
546+
* it could still have required parameterization due to LATERAL refs in
547+
* its tlist.
544548
*/
545549
add_path(baserel, (Path*)
546550
create_foreignscan_path(root,baserel,
@@ -549,7 +553,7 @@ fileGetForeignPaths(PlannerInfo *root,
549553
startup_cost,
550554
total_cost,
551555
NIL,/* no pathkeys */
552-
NULL,/* no outer rel either */
556+
baserel->lateral_relids,
553557
NULL,/* no extra plan */
554558
coptions));
555559

‎contrib/postgres_fdw/expected/postgres_fdw.out

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

34223422
reset enable_hashagg;
3423+
-- bug #15613: bad plan for foreign table scan with lateral reference
3424+
EXPLAIN (VERBOSE, COSTS OFF)
3425+
SELECT ref_0.c2, subq_1.*
3426+
FROM
3427+
"S 1"."T 1" AS ref_0,
3428+
LATERAL (
3429+
SELECT ref_0."C 1" c1, subq_0.*
3430+
FROM (SELECT ref_0.c2, ref_1.c3
3431+
FROM ft1 AS ref_1) AS subq_0
3432+
RIGHT JOIN ft2 AS ref_3 ON (subq_0.c3 = ref_3.c3)
3433+
) AS subq_1
3434+
WHERE ref_0."C 1" < 10 AND subq_1.c3 = '00001'
3435+
ORDER BY ref_0."C 1";
3436+
QUERY PLAN
3437+
---------------------------------------------------------------------------------------------------------
3438+
Nested Loop
3439+
Output: ref_0.c2, ref_0."C 1", (ref_0.c2), ref_1.c3, ref_0."C 1"
3440+
-> Nested Loop
3441+
Output: ref_0.c2, ref_0."C 1", ref_1.c3, (ref_0.c2)
3442+
-> Index Scan using t1_pkey on "S 1"."T 1" ref_0
3443+
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
3444+
Index Cond: (ref_0."C 1" < 10)
3445+
-> Foreign Scan on public.ft1 ref_1
3446+
Output: ref_1.c3, ref_0.c2
3447+
Remote SQL: SELECT c3 FROM "S 1"."T 1" WHERE ((c3 = '00001'::text))
3448+
-> Materialize
3449+
Output: ref_3.c3
3450+
-> Foreign Scan on public.ft2 ref_3
3451+
Output: ref_3.c3
3452+
Remote SQL: SELECT c3 FROM "S 1"."T 1" WHERE ((c3 = '00001'::text))
3453+
(15 rows)
3454+
3455+
SELECT ref_0.c2, subq_1.*
3456+
FROM
3457+
"S 1"."T 1" AS ref_0,
3458+
LATERAL (
3459+
SELECT ref_0."C 1" c1, subq_0.*
3460+
FROM (SELECT ref_0.c2, ref_1.c3
3461+
FROM ft1 AS ref_1) AS subq_0
3462+
RIGHT JOIN ft2 AS ref_3 ON (subq_0.c3 = ref_3.c3)
3463+
) AS subq_1
3464+
WHERE ref_0."C 1" < 10 AND subq_1.c3 = '00001'
3465+
ORDER BY ref_0."C 1";
3466+
c2 | c1 | c2 | c3
3467+
----+----+----+-------
3468+
1 | 1 | 1 | 00001
3469+
2 | 2 | 2 | 00001
3470+
3 | 3 | 3 | 00001
3471+
4 | 4 | 4 | 00001
3472+
5 | 5 | 5 | 00001
3473+
6 | 6 | 6 | 00001
3474+
7 | 7 | 7 | 00001
3475+
8 | 8 | 8 | 00001
3476+
9 | 9 | 9 | 00001
3477+
(9 rows)
3478+
34233479
-- Check with placeHolderVars
34243480
explain (verbose, costs off)
34253481
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: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -897,14 +897,17 @@ postgresGetForeignPaths(PlannerInfo *root,
897897
* baserestrict conditions we were able to send to remote, there might
898898
* actually be an indexscan happening there). We already did all the work
899899
* to estimate cost and size of this path.
900+
*
901+
* Although this path uses no join clauses, it could still have required
902+
* parameterization due to LATERAL refs in its tlist.
900903
*/
901904
path=create_foreignscan_path(root,baserel,
902905
NULL,/* default pathtarget */
903906
fpinfo->rows,
904907
fpinfo->startup_cost,
905908
fpinfo->total_cost,
906909
NIL,/* no pathkeys */
907-
NULL,/* no outer rel either */
910+
baserel->lateral_relids,
908911
NULL,/* no extra plan */
909912
NIL);/* no fdw_private list */
910913
add_path(baserel, (Path*)path);
@@ -4374,7 +4377,7 @@ add_paths_with_pathkeys_for_rel(PlannerInfo *root, RelOptInfo *rel,
43744377
startup_cost,
43754378
total_cost,
43764379
useful_pathkeys,
4377-
NULL,
4380+
rel->lateral_relids,
43784381
sorted_epq_path,
43794382
NIL));
43804383
}
@@ -4511,6 +4514,13 @@ postgresGetForeignJoinPaths(PlannerInfo *root,
45114514
if (joinrel->fdw_private)
45124515
return;
45134516

4517+
/*
4518+
* This code does not work for joins with lateral references, since those
4519+
* must have parameterized paths, which we don't generate yet.
4520+
*/
4521+
if (!bms_is_empty(joinrel->lateral_relids))
4522+
return;
4523+
45144524
/*
45154525
* Create unfinished PgFdwRelationInfo entry which is used to indicate
45164526
* that the join relation is already considered, so that we won't waste
@@ -4602,7 +4612,7 @@ postgresGetForeignJoinPaths(PlannerInfo *root,
46024612
startup_cost,
46034613
total_cost,
46044614
NIL,/* no pathkeys */
4605-
NULL,/* no required_outer */
4615+
joinrel->lateral_relids,
46064616
epq_path,
46074617
NIL);/* no fdw_private */
46084618

@@ -4921,7 +4931,7 @@ add_foreign_grouping_paths(PlannerInfo *root, RelOptInfo *input_rel,
49214931
startup_cost,
49224932
total_cost,
49234933
NIL,/* no pathkeys */
4924-
NULL,/* no required_outer */
4934+
grouped_rel->lateral_relids,
49254935
NULL,
49264936
NIL);/* no fdw_private */
49274937

‎contrib/postgres_fdw/sql/postgres_fdw.sql

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -878,6 +878,32 @@ select c2, sum from "S 1"."T 1" t1, lateral (select sum(t2.c1 + t1."C 1") sum fr
878878
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;
879879
reset enable_hashagg;
880880

881+
-- bug #15613: bad plan for foreign table scan with lateral reference
882+
EXPLAIN (VERBOSE, COSTS OFF)
883+
SELECTref_0.c2, subq_1.*
884+
FROM
885+
"S 1"."T 1"AS ref_0,
886+
LATERAL (
887+
SELECT ref_0."C 1" c1, subq_0.*
888+
FROM (SELECTref_0.c2,ref_1.c3
889+
FROM ft1AS ref_1)AS subq_0
890+
RIGHT JOIN ft2AS ref_3ON (subq_0.c3=ref_3.c3)
891+
)AS subq_1
892+
WHERE ref_0."C 1"<10ANDsubq_1.c3='00001'
893+
ORDER BY ref_0."C 1";
894+
895+
SELECTref_0.c2, subq_1.*
896+
FROM
897+
"S 1"."T 1"AS ref_0,
898+
LATERAL (
899+
SELECT ref_0."C 1" c1, subq_0.*
900+
FROM (SELECTref_0.c2,ref_1.c3
901+
FROM ft1AS ref_1)AS subq_0
902+
RIGHT JOIN ft2AS ref_3ON (subq_0.c3=ref_3.c3)
903+
)AS subq_1
904+
WHERE ref_0."C 1"<10ANDsubq_1.c3='00001'
905+
ORDER BY ref_0."C 1";
906+
881907
-- Check with placeHolderVars
882908
explain (verbose, costs off)
883909
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);

‎src/backend/optimizer/util/pathnode.c

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1967,6 +1967,27 @@ create_foreignscan_path(PlannerInfo *root, RelOptInfo *rel,
19671967
{
19681968
ForeignPath*pathnode=makeNode(ForeignPath);
19691969

1970+
/*
1971+
* Since the path's required_outer should always include all the rel's
1972+
* lateral_relids, forcibly add those if necessary. This is a bit of a
1973+
* hack, but up till early 2019 the contrib FDWs failed to ensure that,
1974+
* and it's likely that the same error has propagated into many external
1975+
* FDWs. Don't risk modifying the passed-in relid set here.
1976+
*/
1977+
if (rel->lateral_relids&& !bms_is_subset(rel->lateral_relids,
1978+
required_outer))
1979+
required_outer=bms_union(required_outer,rel->lateral_relids);
1980+
1981+
/*
1982+
* Although this function is only designed to be used for scans of
1983+
* baserels, before v12 postgres_fdw abused it to make paths for join and
1984+
* upper rels. It will work for such cases as long as required_outer is
1985+
* empty (otherwise get_baserel_parampathinfo does the wrong thing), which
1986+
* fortunately is the expected case for now.
1987+
*/
1988+
if (!bms_is_empty(required_outer)&& !IS_SIMPLE_REL(rel))
1989+
elog(ERROR,"parameterized foreign joins are not supported yet");
1990+
19701991
pathnode->path.pathtype=T_ForeignScan;
19711992
pathnode->path.parent=rel;
19721993
pathnode->path.pathtarget=target ?target :rel->reltarget;

‎src/backend/optimizer/util/relnode.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1041,6 +1041,9 @@ get_baserel_parampathinfo(PlannerInfo *root, RelOptInfo *baserel,
10411041
doublerows;
10421042
ListCell*lc;
10431043

1044+
/* If rel has LATERAL refs, every path for it should account for them */
1045+
Assert(bms_is_subset(baserel->lateral_relids,required_outer));
1046+
10441047
/* Unparameterized paths have no ParamPathInfo */
10451048
if (bms_is_empty(required_outer))
10461049
returnNULL;
@@ -1140,6 +1143,9 @@ get_joinrel_parampathinfo(PlannerInfo *root, RelOptInfo *joinrel,
11401143
doublerows;
11411144
ListCell*lc;
11421145

1146+
/* If rel has LATERAL refs, every path for it should account for them */
1147+
Assert(bms_is_subset(joinrel->lateral_relids,required_outer));
1148+
11431149
/* Unparameterized paths have no ParamPathInfo or extra join clauses */
11441150
if (bms_is_empty(required_outer))
11451151
returnNULL;
@@ -1336,6 +1342,9 @@ get_appendrel_parampathinfo(RelOptInfo *appendrel, Relids required_outer)
13361342
ParamPathInfo*ppi;
13371343
ListCell*lc;
13381344

1345+
/* If rel has LATERAL refs, every path for it should account for them */
1346+
Assert(bms_is_subset(appendrel->lateral_relids,required_outer));
1347+
13391348
/* Unparameterized paths have no ParamPathInfo */
13401349
if (bms_is_empty(required_outer))
13411350
returnNULL;

0 commit comments

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