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

Commit9d6d2b2

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 parent8722d2c commit9d6d2b2

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
@@ -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
@@ -3435,6 +3435,62 @@ select c2, sum from "S 1"."T 1" t1, lateral (select sum(t2.c1 + t1."C 1") sum fr
34353435
(2 rows)
34363436

34373437
reset enable_hashagg;
3438+
-- bug #15613: bad plan for foreign table scan with lateral reference
3439+
EXPLAIN (VERBOSE, COSTS OFF)
3440+
SELECT ref_0.c2, subq_1.*
3441+
FROM
3442+
"S 1"."T 1" AS ref_0,
3443+
LATERAL (
3444+
SELECT ref_0."C 1" c1, subq_0.*
3445+
FROM (SELECT ref_0.c2, ref_1.c3
3446+
FROM ft1 AS ref_1) AS subq_0
3447+
RIGHT JOIN ft2 AS ref_3 ON (subq_0.c3 = ref_3.c3)
3448+
) AS subq_1
3449+
WHERE ref_0."C 1" < 10 AND subq_1.c3 = '00001'
3450+
ORDER BY ref_0."C 1";
3451+
QUERY PLAN
3452+
---------------------------------------------------------------------------------------------------------
3453+
Nested Loop
3454+
Output: ref_0.c2, ref_0."C 1", (ref_0.c2), ref_1.c3, ref_0."C 1"
3455+
-> Nested Loop
3456+
Output: ref_0.c2, ref_0."C 1", ref_1.c3, (ref_0.c2)
3457+
-> Index Scan using t1_pkey on "S 1"."T 1" ref_0
3458+
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
3459+
Index Cond: (ref_0."C 1" < 10)
3460+
-> Foreign Scan on public.ft1 ref_1
3461+
Output: ref_1.c3, ref_0.c2
3462+
Remote SQL: SELECT c3 FROM "S 1"."T 1" WHERE ((c3 = '00001'::text))
3463+
-> Materialize
3464+
Output: ref_3.c3
3465+
-> Foreign Scan on public.ft2 ref_3
3466+
Output: ref_3.c3
3467+
Remote SQL: SELECT c3 FROM "S 1"."T 1" WHERE ((c3 = '00001'::text))
3468+
(15 rows)
3469+
3470+
SELECT ref_0.c2, subq_1.*
3471+
FROM
3472+
"S 1"."T 1" AS ref_0,
3473+
LATERAL (
3474+
SELECT ref_0."C 1" c1, subq_0.*
3475+
FROM (SELECT ref_0.c2, ref_1.c3
3476+
FROM ft1 AS ref_1) AS subq_0
3477+
RIGHT JOIN ft2 AS ref_3 ON (subq_0.c3 = ref_3.c3)
3478+
) AS subq_1
3479+
WHERE ref_0."C 1" < 10 AND subq_1.c3 = '00001'
3480+
ORDER BY ref_0."C 1";
3481+
c2 | c1 | c2 | c3
3482+
----+----+----+-------
3483+
1 | 1 | 1 | 00001
3484+
2 | 2 | 2 | 00001
3485+
3 | 3 | 3 | 00001
3486+
4 | 4 | 4 | 00001
3487+
5 | 5 | 5 | 00001
3488+
6 | 6 | 6 | 00001
3489+
7 | 7 | 7 | 00001
3490+
8 | 8 | 8 | 00001
3491+
9 | 9 | 9 | 00001
3492+
(9 rows)
3493+
34383494
-- Check with placeHolderVars
34393495
explain (verbose, costs off)
34403496
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
@@ -930,14 +930,17 @@ postgresGetForeignPaths(PlannerInfo *root,
930930
* baserestrict conditions we were able to send to remote, there might
931931
* actually be an indexscan happening there). We already did all the work
932932
* to estimate cost and size of this path.
933+
*
934+
* Although this path uses no join clauses, it could still have required
935+
* parameterization due to LATERAL refs in its tlist.
933936
*/
934937
path=create_foreignscan_path(root,baserel,
935938
NULL,/* default pathtarget */
936939
fpinfo->rows,
937940
fpinfo->startup_cost,
938941
fpinfo->total_cost,
939942
NIL,/* no pathkeys */
940-
NULL,/* no outer rel either */
943+
baserel->lateral_relids,
941944
NULL,/* no extra plan */
942945
NIL);/* no fdw_private list */
943946
add_path(baserel, (Path*)path);
@@ -4978,7 +4981,7 @@ add_paths_with_pathkeys_for_rel(PlannerInfo *root, RelOptInfo *rel,
49784981
startup_cost,
49794982
total_cost,
49804983
useful_pathkeys,
4981-
NULL,
4984+
rel->lateral_relids,
49824985
sorted_epq_path,
49834986
NIL));
49844987
}
@@ -5115,6 +5118,13 @@ postgresGetForeignJoinPaths(PlannerInfo *root,
51155118
if (joinrel->fdw_private)
51165119
return;
51175120

5121+
/*
5122+
* This code does not work for joins with lateral references, since those
5123+
* must have parameterized paths, which we don't generate yet.
5124+
*/
5125+
if (!bms_is_empty(joinrel->lateral_relids))
5126+
return;
5127+
51185128
/*
51195129
* Create unfinished PgFdwRelationInfo entry which is used to indicate
51205130
* that the join relation is already considered, so that we won't waste
@@ -5206,7 +5216,7 @@ postgresGetForeignJoinPaths(PlannerInfo *root,
52065216
startup_cost,
52075217
total_cost,
52085218
NIL,/* no pathkeys */
5209-
NULL,/* no required_outer */
5219+
joinrel->lateral_relids,
52105220
epq_path,
52115221
NIL);/* no fdw_private */
52125222

@@ -5534,7 +5544,7 @@ add_foreign_grouping_paths(PlannerInfo *root, RelOptInfo *input_rel,
55345544
startup_cost,
55355545
total_cost,
55365546
NIL,/* no pathkeys */
5537-
NULL,/* no required_outer */
5547+
grouped_rel->lateral_relids,
55385548
NULL,
55395549
NIL);/* no fdw_private */
55405550

‎contrib/postgres_fdw/sql/postgres_fdw.sql‎

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

887+
-- bug #15613: bad plan for foreign table scan with lateral reference
888+
EXPLAIN (VERBOSE, COSTS OFF)
889+
SELECTref_0.c2, subq_1.*
890+
FROM
891+
"S 1"."T 1"AS ref_0,
892+
LATERAL (
893+
SELECT ref_0."C 1" c1, subq_0.*
894+
FROM (SELECTref_0.c2,ref_1.c3
895+
FROM ft1AS ref_1)AS subq_0
896+
RIGHT JOIN ft2AS ref_3ON (subq_0.c3=ref_3.c3)
897+
)AS subq_1
898+
WHERE ref_0."C 1"<10ANDsubq_1.c3='00001'
899+
ORDER BY ref_0."C 1";
900+
901+
SELECTref_0.c2, subq_1.*
902+
FROM
903+
"S 1"."T 1"AS ref_0,
904+
LATERAL (
905+
SELECT ref_0."C 1" c1, subq_0.*
906+
FROM (SELECTref_0.c2,ref_1.c3
907+
FROM ft1AS ref_1)AS subq_0
908+
RIGHT JOIN ft2AS ref_3ON (subq_0.c3=ref_3.c3)
909+
)AS subq_1
910+
WHERE ref_0."C 1"<10ANDsubq_1.c3='00001'
911+
ORDER BY ref_0."C 1";
912+
887913
-- Check with placeHolderVars
888914
explain (verbose, costs off)
889915
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
@@ -2069,6 +2069,27 @@ create_foreignscan_path(PlannerInfo *root, RelOptInfo *rel,
20692069
{
20702070
ForeignPath*pathnode=makeNode(ForeignPath);
20712071

2072+
/*
2073+
* Since the path's required_outer should always include all the rel's
2074+
* lateral_relids, forcibly add those if necessary. This is a bit of a
2075+
* hack, but up till early 2019 the contrib FDWs failed to ensure that,
2076+
* and it's likely that the same error has propagated into many external
2077+
* FDWs. Don't risk modifying the passed-in relid set here.
2078+
*/
2079+
if (rel->lateral_relids&& !bms_is_subset(rel->lateral_relids,
2080+
required_outer))
2081+
required_outer=bms_union(required_outer,rel->lateral_relids);
2082+
2083+
/*
2084+
* Although this function is only designed to be used for scans of
2085+
* baserels, before v12 postgres_fdw abused it to make paths for join and
2086+
* upper rels. It will work for such cases as long as required_outer is
2087+
* empty (otherwise get_baserel_parampathinfo does the wrong thing), which
2088+
* fortunately is the expected case for now.
2089+
*/
2090+
if (!bms_is_empty(required_outer)&& !IS_SIMPLE_REL(rel))
2091+
elog(ERROR,"parameterized foreign joins are not supported yet");
2092+
20722093
pathnode->path.pathtype=T_ForeignScan;
20732094
pathnode->path.parent=rel;
20742095
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
@@ -1247,6 +1247,9 @@ get_baserel_parampathinfo(PlannerInfo *root, RelOptInfo *baserel,
12471247
doublerows;
12481248
ListCell*lc;
12491249

1250+
/* If rel has LATERAL refs, every path for it should account for them */
1251+
Assert(bms_is_subset(baserel->lateral_relids,required_outer));
1252+
12501253
/* Unparameterized paths have no ParamPathInfo */
12511254
if (bms_is_empty(required_outer))
12521255
returnNULL;
@@ -1342,6 +1345,9 @@ get_joinrel_parampathinfo(PlannerInfo *root, RelOptInfo *joinrel,
13421345
doublerows;
13431346
ListCell*lc;
13441347

1348+
/* If rel has LATERAL refs, every path for it should account for them */
1349+
Assert(bms_is_subset(joinrel->lateral_relids,required_outer));
1350+
13451351
/* Unparameterized paths have no ParamPathInfo or extra join clauses */
13461352
if (bms_is_empty(required_outer))
13471353
returnNULL;
@@ -1533,6 +1539,9 @@ get_appendrel_parampathinfo(RelOptInfo *appendrel, Relids required_outer)
15331539
{
15341540
ParamPathInfo*ppi;
15351541

1542+
/* If rel has LATERAL refs, every path for it should account for them */
1543+
Assert(bms_is_subset(appendrel->lateral_relids,required_outer));
1544+
15361545
/* Unparameterized paths have no ParamPathInfo */
15371546
if (bms_is_empty(required_outer))
15381547
returnNULL;

0 commit comments

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