Skip to content

Commit 4526951

Browse files
tglsfdctglsfdc
committed
Make postgres_fdw's "Relations" output agree with the rest of EXPLAIN.
The relation aliases shown in the "Relations" line for a foreign scan didn't always agree with those used in the rest of EXPLAIN's output. The regression test result changes appearing here provide examples. It's really impossible for postgres_fdw to duplicate EXPLAIN's alias assignment logic during postgresGetForeignRelSize(), because of the de-duplication that EXPLAIN does on a global basis --- and anyway, trying to duplicate that would be unmaintainable. Instead, just put numeric rangetable indexes into the string, and convert those to table names/aliases in postgresExplainForeignScan, which does have access to the results of ruleutils.c's alias assignment logic. Aside from being more reliable, this shifts some work from planning to EXPLAIN, which is a good tradeoff for performance. (I also changed from using StringInfo to using psprintf, which makes the code slightly simpler and reduces its memory consumption.) A kluge required by this solution is that we have to reverse-engineer the rtoffset applied by setrefs.c. If that logic ever fails (presumably because the member tables of a join got offset by different amounts), we'll need some more cooperation with setrefs.c to keep things straight. But for now, there's no need for that. Arguably this is a back-patchable bug fix, but since this is a mostly cosmetic issue and there have been no field complaints, I'll refrain for now. Discussion: https://postgr.es/m/12424.1575168015@sss.pgh.pa.us
1 parent 1d468b9 commit 4526951

File tree

3 files changed

+117
-56
lines changed

3 files changed

+117
-56
lines changed

‎contrib/postgres_fdw/expected/postgres_fdw.out

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1348,7 +1348,7 @@ SELECT t1.c1, ss.a, ss.b FROM (SELECT c1 FROM ft4 WHERE c1 between 50 and 60) t1
13481348
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
13491349
Foreign Scan
13501350
Output: ft4.c1, ft4_1.c1, ft5.c1
1351-
Relations: (public.ft4) FULL JOIN ((public.ft4) FULL JOIN (public.ft5))
1351+
Relations: (public.ft4) FULL JOIN ((public.ft4 ft4_1) FULL JOIN (public.ft5))
13521352
Remote SQL: SELECT s4.c1, s10.c1, s10.c2 FROM ((SELECT c1 FROM "S 1"."T 3" WHERE ((c1 >= 50)) AND ((c1 <= 60))) s4(c1) FULL JOIN (SELECT s8.c1, s9.c1 FROM ((SELECT c1 FROM "S 1"."T 3" WHERE ((c1 >= 50)) AND ((c1 <= 60))) s8(c1) FULL JOIN (SELECT c1 FROM "S 1"."T 4" WHERE ((c1 >= 50)) AND ((c1 <= 60))) s9(c1) ON (((s8.c1 = s9.c1)))) WHERE (((s8.c1 IS NULL) OR (s8.c1 IS NOT NULL)))) s10(c1, c2) ON (((s4.c1 = s10.c1)))) ORDER BY s4.c1 ASC NULLS LAST, s10.c1 ASC NULLS LAST, s10.c2 ASC NULLS LAST
13531353
(4 rows)
13541354

@@ -2084,7 +2084,7 @@ SELECT t1c1, avg(t1c1 + t2c1) FROM (SELECT t1.c1, t2.c1 FROM ft1 t1 JOIN ft2 t2
20842084
Remote SQL: SELECT r1."C 1", r2."C 1" FROM ("S 1"."T 1" r1 INNER JOIN "S 1"."T 1" r2 ON (((r1."C 1" = r2."C 1"))))
20852085
-> Foreign Scan
20862086
Output: t1_1.c1, t2_1.c1
2087-
Relations: (public.ft1 t1) INNER JOIN (public.ft2 t2)
2087+
Relations: (public.ft1 t1_1) INNER JOIN (public.ft2 t2_1)
20882088
Remote SQL: SELECT r1."C 1", r2."C 1" FROM ("S 1"."T 1" r1 INNER JOIN "S 1"."T 1" r2 ON (((r1."C 1" = r2."C 1"))))
20892089
(20 rows)
20902090

@@ -3230,7 +3230,7 @@ select count(*), x.b from ft1, (select c2 a, sum(c1) b from ft1 group by c2) x w
32303230
Output: x.b, x.a
32313231
-> Foreign Scan
32323232
Output: ft1_1.c2, (sum(ft1_1.c1))
3233-
Relations: Aggregate on (public.ft1)
3233+
Relations: Aggregate on (public.ft1 ft1_1)
32343234
Remote SQL: SELECT c2, sum("C 1") FROM "S 1"."T 1" GROUP BY 1
32353235
(21 rows)
32363236

@@ -8480,15 +8480,15 @@ ANALYZE fprt2_p2;
84808480
-- inner join three tables
84818481
EXPLAIN (COSTS OFF)
84828482
SELECT t1.a,t2.b,t3.c FROM fprt1 t1 INNER JOIN fprt2 t2 ON (t1.a = t2.b) INNER JOIN fprt1 t3 ON (t2.b = t3.a) WHERE t1.a % 25 =0 ORDER BY 1,2,3;
8483-
QUERY PLAN
8484-
--------------------------------------------------------------------------------------------------------------------
8483+
QUERY PLAN
8484+
--------------------------------------------------------------------------------------------------------------------------
84858485
Sort
84868486
Sort Key: t1.a, t3.c
84878487
-> Append
84888488
-> Foreign Scan
84898489
Relations: ((public.ftprt1_p1 t1) INNER JOIN (public.ftprt2_p1 t2)) INNER JOIN (public.ftprt1_p1 t3)
84908490
-> Foreign Scan
8491-
Relations: ((public.ftprt1_p2 t1) INNER JOIN (public.ftprt2_p2 t2)) INNER JOIN (public.ftprt1_p2 t3)
8491+
Relations: ((public.ftprt1_p2 t1_1) INNER JOIN (public.ftprt2_p2 t2_1)) INNER JOIN (public.ftprt1_p2 t3_1)
84928492
(7 rows)
84938493

84948494
SELECT t1.a,t2.b,t3.c FROM fprt1 t1 INNER JOIN fprt2 t2 ON (t1.a = t2.b) INNER JOIN fprt1 t3 ON (t2.b = t3.a) WHERE t1.a % 25 =0 ORDER BY 1,2,3;
@@ -8507,7 +8507,7 @@ SELECT t1.a,t2.b,t2.c FROM fprt1 t1 LEFT JOIN (SELECT * FROM fprt2 WHERE a < 10)
85078507
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
85088508
Foreign Scan
85098509
Output: t1.a, ftprt2_p1.b, ftprt2_p1.c
8510-
Relations: (public.ftprt1_p1 t1) LEFT JOIN (public.ftprt2_p1 fprt2)
8510+
Relations: (public.ftprt1_p1 t1) LEFT JOIN (public.ftprt2_p1)
85118511
Remote SQL: SELECT r5.a, r6.b, r6.c FROM (public.fprt1_p1 r5 LEFT JOIN public.fprt2_p1 r6 ON (((r5.a = r6.b)) AND ((r5.b = r6.a)) AND ((r6.a < 10)))) WHERE ((r5.a < 10)) ORDER BY r5.a ASC NULLS LAST, r6.b ASC NULLS LAST, r6.c ASC NULLS LAST
85128512
(4 rows)
85138513

@@ -8561,15 +8561,15 @@ SELECT t1.wr, t2.wr FROM (SELECT t1 wr, a FROM fprt1 t1 WHERE t1.a % 25 = 0) t1
85618561
-- join with lateral reference
85628562
EXPLAIN (COSTS OFF)
85638563
SELECT t1.a,t1.b FROM fprt1 t1, LATERAL (SELECT t2.a, t2.b FROM fprt2 t2 WHERE t1.a = t2.b AND t1.b = t2.a) q WHERE t1.a%25 = 0 ORDER BY 1,2;
8564-
QUERY PLAN
8565-
---------------------------------------------------------------------------------
8564+
QUERY PLAN
8565+
-------------------------------------------------------------------------------------
85668566
Sort
85678567
Sort Key: t1.a, t1.b
85688568
-> Append
85698569
-> Foreign Scan
85708570
Relations: (public.ftprt1_p1 t1) INNER JOIN (public.ftprt2_p1 t2)
85718571
-> Foreign Scan
8572-
Relations: (public.ftprt1_p2 t1) INNER JOIN (public.ftprt2_p2 t2)
8572+
Relations: (public.ftprt1_p2 t1_1) INNER JOIN (public.ftprt2_p2 t2_1)
85738573
(7 rows)
85748574

85758575
SELECT t1.a,t1.b FROM fprt1 t1, LATERAL (SELECT t2.a, t2.b FROM fprt2 t2 WHERE t1.a = t2.b AND t1.b = t2.a) q WHERE t1.a%25 = 0 ORDER BY 1,2;
@@ -8689,17 +8689,17 @@ SELECT a, sum(b), min(b), count(*) FROM pagg_tab GROUP BY a HAVING avg(b) < 22 O
86898689
SET enable_partitionwise_aggregate TO true;
86908690
EXPLAIN (COSTS OFF)
86918691
SELECT a, sum(b), min(b), count(*) FROM pagg_tab GROUP BY a HAVING avg(b) < 22 ORDER BY 1;
8692-
QUERY PLAN
8693-
----------------------------------------------------------------------
8692+
QUERY PLAN
8693+
-------------------------------------------------------------
86948694
Sort
86958695
Sort Key: fpagg_tab_p1.a
86968696
-> Append
86978697
-> Foreign Scan
8698-
Relations: Aggregate on (public.fpagg_tab_p1 pagg_tab)
8698+
Relations: Aggregate on (public.fpagg_tab_p1)
86998699
-> Foreign Scan
8700-
Relations: Aggregate on (public.fpagg_tab_p2 pagg_tab)
8700+
Relations: Aggregate on (public.fpagg_tab_p2)
87018701
-> Foreign Scan
8702-
Relations: Aggregate on (public.fpagg_tab_p3 pagg_tab)
8702+
Relations: Aggregate on (public.fpagg_tab_p3)
87038703
(9 rows)
87048704

87058705
SELECT a, sum(b), min(b), count(*) FROM pagg_tab GROUP BY a HAVING avg(b) < 22 ORDER BY 1;

‎contrib/postgres_fdw/postgres_fdw.c

Lines changed: 95 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
*/
1313
#include "postgres.h"
1414

15+
#include <limits.h>
16+
1517
#include "access/htup_details.h"
1618
#include "access/sysattr.h"
1719
#include "access/table.h"
@@ -574,9 +576,6 @@ postgresGetForeignRelSize(PlannerInfo *root,
574576
PgFdwRelationInfo *fpinfo;
575577
ListCell *lc;
576578
RangeTblEntry *rte = planner_rt_fetch(baserel->relid, root);
577-
const char *namespace;
578-
const char *relname;
579-
const char *refname;
580579

581580
/*
582581
* We use PgFdwRelationInfo to pass various information to subsequent
@@ -719,21 +718,11 @@ postgresGetForeignRelSize(PlannerInfo *root,
719718
}
720719

721720
/*
722-
* Set the name of relation in fpinfo, while we are constructing it here.
723-
* It will be used to build the string describing the join relation in
724-
* EXPLAIN output. We can't know whether VERBOSE option is specified or
725-
* not, so always schema-qualify the foreign table name.
721+
* fpinfo->relation_name gets the numeric rangetable index of the foreign
722+
* table RTE. (If this query gets EXPLAIN'd, we'll convert that to a
723+
* human-readable string at that time.)
726724
*/
727-
fpinfo->relation_name = makeStringInfo();
728-
namespace = get_namespace_name(get_rel_namespace(foreigntableid));
729-
relname = get_rel_name(foreigntableid);
730-
refname = rte->eref->aliasname;
731-
appendStringInfo(fpinfo->relation_name, "%s.%s",
732-
quote_identifier(namespace),
733-
quote_identifier(relname));
734-
if (*refname && strcmp(refname, relname) != 0)
735-
appendStringInfo(fpinfo->relation_name, " %s",
736-
quote_identifier(rte->eref->aliasname));
725+
fpinfo->relation_name = psprintf("%u", baserel->relid);
737726

738727
/* No outer and inner relations. */
739728
fpinfo->make_outerrel_subquery = false;
@@ -1376,7 +1365,7 @@ postgresGetForeignPlan(PlannerInfo *root,
13761365
makeInteger(fpinfo->fetch_size));
13771366
if (IS_JOIN_REL(foreignrel) || IS_UPPER_REL(foreignrel))
13781367
fdw_private = lappend(fdw_private,
1379-
makeString(fpinfo->relation_name->data));
1368+
makeString(fpinfo->relation_name));
13801369

13811370
/*
13821371
* Create the ForeignScan node for the given relation.
@@ -2528,27 +2517,94 @@ postgresEndDirectModify(ForeignScanState *node)
25282517
static void
25292518
postgresExplainForeignScan(ForeignScanState *node, ExplainState *es)
25302519
{
2531-
List *fdw_private;
2532-
char *sql;
2533-
char *relations;
2534-
2535-
fdw_private = ((ForeignScan *) node->ss.ps.plan)->fdw_private;
2520+
ForeignScan *plan = castNode(ForeignScan, node->ss.ps.plan);
2521+
List *fdw_private = plan->fdw_private;
25362522

25372523
/*
2538-
* Add names of relation handled by the foreign scan when the scan is a
2539-
* join
2524+
* Identify foreign scans that are really joins or upper relations. The
2525+
* input looks something like "(1) LEFT JOIN (2)", and we must replace the
2526+
* digit string(s), which are RT indexes, with the correct relation names.
2527+
* We do that here, not when the plan is created, because we can't know
2528+
* what aliases ruleutils.c will assign at plan creation time.
25402529
*/
25412530
if (list_length(fdw_private) > FdwScanPrivateRelations)
25422531
{
2543-
relations = strVal(list_nth(fdw_private, FdwScanPrivateRelations));
2544-
ExplainPropertyText("Relations", relations, es);
2532+
StringInfo relations;
2533+
char *rawrelations;
2534+
char *ptr;
2535+
int minrti,
2536+
rtoffset;
2537+
2538+
rawrelations = strVal(list_nth(fdw_private, FdwScanPrivateRelations));
2539+
2540+
/*
2541+
* A difficulty with using a string representation of RT indexes is
2542+
* that setrefs.c won't update the string when flattening the
2543+
* rangetable. To find out what rtoffset was applied, identify the
2544+
* minimum RT index appearing in the string and compare it to the
2545+
* minimum member of plan->fs_relids. (We expect all the relids in
2546+
* the join will have been offset by the same amount; the Asserts
2547+
* below should catch it if that ever changes.)
2548+
*/
2549+
minrti = INT_MAX;
2550+
ptr = rawrelations;
2551+
while (*ptr)
2552+
{
2553+
if (isdigit((unsigned char) *ptr))
2554+
{
2555+
int rti = strtol(ptr, &ptr, 10);
2556+
2557+
if (rti < minrti)
2558+
minrti = rti;
2559+
}
2560+
else
2561+
ptr++;
2562+
}
2563+
rtoffset = bms_next_member(plan->fs_relids, -1) - minrti;
2564+
2565+
/* Now we can translate the string */
2566+
relations = makeStringInfo();
2567+
ptr = rawrelations;
2568+
while (*ptr)
2569+
{
2570+
if (isdigit((unsigned char) *ptr))
2571+
{
2572+
int rti = strtol(ptr, &ptr, 10);
2573+
RangeTblEntry *rte;
2574+
char *namespace;
2575+
char *relname;
2576+
char *refname;
2577+
2578+
rti += rtoffset;
2579+
Assert(bms_is_member(rti, plan->fs_relids));
2580+
rte = rt_fetch(rti, es->rtable);
2581+
Assert(rte->rtekind == RTE_RELATION);
2582+
/* This logic should agree with explain.c's ExplainTargetRel */
2583+
namespace = get_namespace_name(get_rel_namespace(rte->relid));
2584+
relname = get_rel_name(rte->relid);
2585+
appendStringInfo(relations, "%s.%s",
2586+
quote_identifier(namespace),
2587+
quote_identifier(relname));
2588+
refname = (char *) list_nth(es->rtable_names, rti - 1);
2589+
if (refname == NULL)
2590+
refname = rte->eref->aliasname;
2591+
if (strcmp(refname, relname) != 0)
2592+
appendStringInfo(relations, " %s",
2593+
quote_identifier(refname));
2594+
}
2595+
else
2596+
appendStringInfoChar(relations, *ptr++);
2597+
}
2598+
ExplainPropertyText("Relations", relations->data, es);
25452599
}
25462600

25472601
/*
25482602
* Add remote query, when VERBOSE option is specified.
25492603
*/
25502604
if (es->verbose)
25512605
{
2606+
char *sql;
2607+
25522608
sql = strVal(list_nth(fdw_private, FdwScanPrivateSelectSql));
25532609
ExplainPropertyText("Remote SQL", sql, es);
25542610
}
@@ -5171,13 +5227,14 @@ foreign_join_ok(PlannerInfo *root, RelOptInfo *joinrel, JoinType jointype,
51715227

51725228
/*
51735229
* Set the string describing this join relation to be used in EXPLAIN
5174-
* output of corresponding ForeignScan.
5230+
* output of corresponding ForeignScan. Note that the decoration we add
5231+
* to the base relation names mustn't include any digits, or it'll confuse
5232+
* postgresExplainForeignScan.
51755233
*/
5176-
fpinfo->relation_name = makeStringInfo();
5177-
appendStringInfo(fpinfo->relation_name, "(%s) %s JOIN (%s)",
5178-
fpinfo_o->relation_name->data,
5179-
get_jointype_name(fpinfo->jointype),
5180-
fpinfo_i->relation_name->data);
5234+
fpinfo->relation_name = psprintf("(%s) %s JOIN (%s)",
5235+
fpinfo_o->relation_name,
5236+
get_jointype_name(fpinfo->jointype),
5237+
fpinfo_i->relation_name);
51815238

51825239
/*
51835240
* Set the relation index. This is defined as the position of this
@@ -5723,11 +5780,12 @@ foreign_grouping_ok(PlannerInfo *root, RelOptInfo *grouped_rel,
57235780

57245781
/*
57255782
* Set the string describing this grouped relation to be used in EXPLAIN
5726-
* output of corresponding ForeignScan.
5783+
* output of corresponding ForeignScan. Note that the decoration we add
5784+
* to the base relation name mustn't include any digits, or it'll confuse
5785+
* postgresExplainForeignScan.
57275786
*/
5728-
fpinfo->relation_name = makeStringInfo();
5729-
appendStringInfo(fpinfo->relation_name, "Aggregate on (%s)",
5730-
ofpinfo->relation_name->data);
5787+
fpinfo->relation_name = psprintf("Aggregate on (%s)",
5788+
ofpinfo->relation_name);
57315789

57325790
return true;
57335791
}

‎contrib/postgres_fdw/postgres_fdw.h

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,11 +87,14 @@ typedef struct PgFdwRelationInfo
8787
int fetch_size; /* fetch size for this remote table */
8888

8989
/*
90-
* Name of the relation while EXPLAINing ForeignScan. It is used for join
91-
* relations but is set for all relations. For join relation, the name
92-
* indicates which foreign tables are being joined and the join type used.
90+
* Name of the relation, for use while EXPLAINing ForeignScan. It is used
91+
* for join and upper relations but is set for all relations. For a base
92+
* relation, this is really just the RT index as a string; we convert that
93+
* while producing EXPLAIN output. For join and upper relations, the name
94+
* indicates which base foreign tables are included and the join type or
95+
* aggregation type used.
9396
*/
94-
StringInfo relation_name;
97+
char *relation_name;
9598

9699
/* Join information */
97100
RelOptInfo *outerrel;

0 commit comments

Comments
 (0)