Make postgres_fdw's "Relations" output agree with the rest of EXPLAIN.
authorTom Lane <[email protected]>
Mon, 2 Dec 2019 21:31:03 +0000 (16:31 -0500)
committerTom Lane <[email protected]>
Mon, 2 Dec 2019 21:31:03 +0000 (16:31 -0500)
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-able 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

contrib/postgres_fdw/expected/postgres_fdw.out
contrib/postgres_fdw/postgres_fdw.c
contrib/postgres_fdw/postgres_fdw.h

index 14180ee46983b94e9ab38613b0c7ed350e7aa755..d2765456384bdfbbc6a9329aa586e949bac9c784 100644 (file)
@@ -1348,7 +1348,7 @@ SELECT t1.c1, ss.a, ss.b FROM (SELECT c1 FROM ft4 WHERE c1 between 50 and 60) t1
 --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  Foreign Scan
    Output: ft4.c1, ft4_1.c1, ft5.c1
-   Relations: (public.ft4) FULL JOIN ((public.ft4) FULL JOIN (public.ft5))
+   Relations: (public.ft4) FULL JOIN ((public.ft4 ft4_1) FULL JOIN (public.ft5))
    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
 (4 rows)
 
@@ -2084,7 +2084,7 @@ SELECT t1c1, avg(t1c1 + t2c1) FROM (SELECT t1.c1, t2.c1 FROM ft1 t1 JOIN ft2 t2
                                  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"))))
                            ->  Foreign Scan
                                  Output: t1_1.c1, t2_1.c1
-                                 Relations: (public.ft1 t1) INNER JOIN (public.ft2 t2)
+                                 Relations: (public.ft1 t1_1) INNER JOIN (public.ft2 t2_1)
                                  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"))))
 (20 rows)
 
@@ -3230,7 +3230,7 @@ select count(*), x.b from ft1, (select c2 a, sum(c1) b from ft1 group by c2) x w
                            Output: x.b, x.a
                            ->  Foreign Scan
                                  Output: ft1_1.c2, (sum(ft1_1.c1))
-                                 Relations: Aggregate on (public.ft1)
+                                 Relations: Aggregate on (public.ft1 ft1_1)
                                  Remote SQL: SELECT c2, sum("C 1") FROM "S 1"."T 1" GROUP BY 1
 (21 rows)
 
@@ -8480,15 +8480,15 @@ ANALYZE fprt2_p2;
 -- inner join three tables
 EXPLAIN (COSTS OFF)
 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;
-                                                     QUERY PLAN                                                     
---------------------------------------------------------------------------------------------------------------------
+                                                        QUERY PLAN                                                        
+--------------------------------------------------------------------------------------------------------------------------
  Sort
    Sort Key: t1.a, t3.c
    ->  Append
          ->  Foreign Scan
                Relations: ((public.ftprt1_p1 t1) INNER JOIN (public.ftprt2_p1 t2)) INNER JOIN (public.ftprt1_p1 t3)
          ->  Foreign Scan
-               Relations: ((public.ftprt1_p2 t1) INNER JOIN (public.ftprt2_p2 t2)) INNER JOIN (public.ftprt1_p2 t3)
+               Relations: ((public.ftprt1_p2 t1_1) INNER JOIN (public.ftprt2_p2 t2_1)) INNER JOIN (public.ftprt1_p2 t3_1)
 (7 rows)
 
 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)
 ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  Foreign Scan
    Output: t1.a, ftprt2_p1.b, ftprt2_p1.c
-   Relations: (public.ftprt1_p1 t1) LEFT JOIN (public.ftprt2_p1 fprt2)
+   Relations: (public.ftprt1_p1 t1) LEFT JOIN (public.ftprt2_p1)
    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
 (4 rows)
 
@@ -8561,15 +8561,15 @@ SELECT t1.wr, t2.wr FROM (SELECT t1 wr, a FROM fprt1 t1 WHERE t1.a % 25 = 0) t1
 -- join with lateral reference
 EXPLAIN (COSTS OFF)
 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;
-                                   QUERY PLAN                                    
----------------------------------------------------------------------------------
+                                     QUERY PLAN                                      
+-------------------------------------------------------------------------------------
  Sort
    Sort Key: t1.a, t1.b
    ->  Append
          ->  Foreign Scan
                Relations: (public.ftprt1_p1 t1) INNER JOIN (public.ftprt2_p1 t2)
          ->  Foreign Scan
-               Relations: (public.ftprt1_p2 t1) INNER JOIN (public.ftprt2_p2 t2)
+               Relations: (public.ftprt1_p2 t1_1) INNER JOIN (public.ftprt2_p2 t2_1)
 (7 rows)
 
 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
 SET enable_partitionwise_aggregate TO true;
 EXPLAIN (COSTS OFF)
 SELECT a, sum(b), min(b), count(*) FROM pagg_tab GROUP BY a HAVING avg(b) < 22 ORDER BY 1;
-                              QUERY PLAN                              
-----------------------------------------------------------------------
+                         QUERY PLAN                          
+-------------------------------------------------------------
  Sort
    Sort Key: fpagg_tab_p1.a
    ->  Append
          ->  Foreign Scan
-               Relations: Aggregate on (public.fpagg_tab_p1 pagg_tab)
+               Relations: Aggregate on (public.fpagg_tab_p1)
          ->  Foreign Scan
-               Relations: Aggregate on (public.fpagg_tab_p2 pagg_tab)
+               Relations: Aggregate on (public.fpagg_tab_p2)
          ->  Foreign Scan
-               Relations: Aggregate on (public.fpagg_tab_p3 pagg_tab)
+               Relations: Aggregate on (public.fpagg_tab_p3)
 (9 rows)
 
 SELECT a, sum(b), min(b), count(*) FROM pagg_tab GROUP BY a HAVING avg(b) < 22 ORDER BY 1;
index fa142960eb55d245e36b86d45935fd40a909992d..3eb4e4044d4e052d77612d99729c6143775203cf 100644 (file)
@@ -12,6 +12,8 @@
  */
 #include "postgres.h"
 
+#include <limits.h>
+
 #include "access/htup_details.h"
 #include "access/sysattr.h"
 #include "access/table.h"
@@ -574,9 +576,6 @@ postgresGetForeignRelSize(PlannerInfo *root,
        PgFdwRelationInfo *fpinfo;
        ListCell   *lc;
        RangeTblEntry *rte = planner_rt_fetch(baserel->relid, root);
-       const char *namespace;
-       const char *relname;
-       const char *refname;
 
        /*
         * We use PgFdwRelationInfo to pass various information to subsequent
@@ -719,21 +718,11 @@ postgresGetForeignRelSize(PlannerInfo *root,
        }
 
        /*
-        * Set the name of relation in fpinfo, while we are constructing it here.
-        * It will be used to build the string describing the join relation in
-        * EXPLAIN output. We can't know whether VERBOSE option is specified or
-        * not, so always schema-qualify the foreign table name.
+        * fpinfo->relation_name gets the numeric rangetable index of the foreign
+        * table RTE.  (If this query gets EXPLAIN'd, we'll convert that to a
+        * human-readable string at that time.)
         */
-       fpinfo->relation_name = makeStringInfo();
-       namespace = get_namespace_name(get_rel_namespace(foreigntableid));
-       relname = get_rel_name(foreigntableid);
-       refname = rte->eref->aliasname;
-       appendStringInfo(fpinfo->relation_name, "%s.%s",
-                                        quote_identifier(namespace),
-                                        quote_identifier(relname));
-       if (*refname && strcmp(refname, relname) != 0)
-               appendStringInfo(fpinfo->relation_name, " %s",
-                                                quote_identifier(rte->eref->aliasname));
+       fpinfo->relation_name = psprintf("%u", baserel->relid);
 
        /* No outer and inner relations. */
        fpinfo->make_outerrel_subquery = false;
@@ -1376,7 +1365,7 @@ postgresGetForeignPlan(PlannerInfo *root,
                                                         makeInteger(fpinfo->fetch_size));
        if (IS_JOIN_REL(foreignrel) || IS_UPPER_REL(foreignrel))
                fdw_private = lappend(fdw_private,
-                                                         makeString(fpinfo->relation_name->data));
+                                                         makeString(fpinfo->relation_name));
 
        /*
         * Create the ForeignScan node for the given relation.
@@ -2528,20 +2517,85 @@ postgresEndDirectModify(ForeignScanState *node)
 static void
 postgresExplainForeignScan(ForeignScanState *node, ExplainState *es)
 {
-       List       *fdw_private;
-       char       *sql;
-       char       *relations;
-
-       fdw_private = ((ForeignScan *) node->ss.ps.plan)->fdw_private;
+       ForeignScan *plan = castNode(ForeignScan, node->ss.ps.plan);
+       List       *fdw_private = plan->fdw_private;
 
        /*
-        * Add names of relation handled by the foreign scan when the scan is a
-        * join
+        * Identify foreign scans that are really joins or upper relations.  The
+        * input looks something like "(1) LEFT JOIN (2)", and we must replace the
+        * digit string(s), which are RT indexes, with the correct relation names.
+        * We do that here, not when the plan is created, because we can't know
+        * what aliases ruleutils.c will assign at plan creation time.
         */
        if (list_length(fdw_private) > FdwScanPrivateRelations)
        {
-               relations = strVal(list_nth(fdw_private, FdwScanPrivateRelations));
-               ExplainPropertyText("Relations", relations, es);
+               StringInfo      relations;
+               char       *rawrelations;
+               char       *ptr;
+               int                     minrti,
+                                       rtoffset;
+
+               rawrelations = strVal(list_nth(fdw_private, FdwScanPrivateRelations));
+
+               /*
+                * A difficulty with using a string representation of RT indexes is
+                * that setrefs.c won't update the string when flattening the
+                * rangetable.  To find out what rtoffset was applied, identify the
+                * minimum RT index appearing in the string and compare it to the
+                * minimum member of plan->fs_relids.  (We expect all the relids in
+                * the join will have been offset by the same amount; the Asserts
+                * below should catch it if that ever changes.)
+                */
+               minrti = INT_MAX;
+               ptr = rawrelations;
+               while (*ptr)
+               {
+                       if (isdigit((unsigned char) *ptr))
+                       {
+                               int                     rti = strtol(ptr, &ptr, 10);
+
+                               if (rti < minrti)
+                                       minrti = rti;
+                       }
+                       else
+                               ptr++;
+               }
+               rtoffset = bms_next_member(plan->fs_relids, -1) - minrti;
+
+               /* Now we can translate the string */
+               relations = makeStringInfo();
+               ptr = rawrelations;
+               while (*ptr)
+               {
+                       if (isdigit((unsigned char) *ptr))
+                       {
+                               int                     rti = strtol(ptr, &ptr, 10);
+                               RangeTblEntry *rte;
+                               char       *namespace;
+                               char       *relname;
+                               char       *refname;
+
+                               rti += rtoffset;
+                               Assert(bms_is_member(rti, plan->fs_relids));
+                               rte = rt_fetch(rti, es->rtable);
+                               Assert(rte->rtekind == RTE_RELATION);
+                               /* This logic should agree with explain.c's ExplainTargetRel */
+                               namespace = get_namespace_name(get_rel_namespace(rte->relid));
+                               relname = get_rel_name(rte->relid);
+                               appendStringInfo(relations, "%s.%s",
+                                                                quote_identifier(namespace),
+                                                                quote_identifier(relname));
+                               refname = (char *) list_nth(es->rtable_names, rti - 1);
+                               if (refname == NULL)
+                                       refname = rte->eref->aliasname;
+                               if (strcmp(refname, relname) != 0)
+                                       appendStringInfo(relations, " %s",
+                                                                        quote_identifier(refname));
+                       }
+                       else
+                               appendStringInfoChar(relations, *ptr++);
+               }
+               ExplainPropertyText("Relations", relations->data, es);
        }
 
        /*
@@ -2549,6 +2603,8 @@ postgresExplainForeignScan(ForeignScanState *node, ExplainState *es)
         */
        if (es->verbose)
        {
+               char       *sql;
+
                sql = strVal(list_nth(fdw_private, FdwScanPrivateSelectSql));
                ExplainPropertyText("Remote SQL", sql, es);
        }
@@ -5171,13 +5227,14 @@ foreign_join_ok(PlannerInfo *root, RelOptInfo *joinrel, JoinType jointype,
 
        /*
         * Set the string describing this join relation to be used in EXPLAIN
-        * output of corresponding ForeignScan.
+        * output of corresponding ForeignScan.  Note that the decoration we add
+        * to the base relation names mustn't include any digits, or it'll confuse
+        * postgresExplainForeignScan.
         */
-       fpinfo->relation_name = makeStringInfo();
-       appendStringInfo(fpinfo->relation_name, "(%s) %s JOIN (%s)",
-                                        fpinfo_o->relation_name->data,
-                                        get_jointype_name(fpinfo->jointype),
-                                        fpinfo_i->relation_name->data);
+       fpinfo->relation_name = psprintf("(%s) %s JOIN (%s)",
+                                                                        fpinfo_o->relation_name,
+                                                                        get_jointype_name(fpinfo->jointype),
+                                                                        fpinfo_i->relation_name);
 
        /*
         * Set the relation index.  This is defined as the position of this
@@ -5723,11 +5780,12 @@ foreign_grouping_ok(PlannerInfo *root, RelOptInfo *grouped_rel,
 
        /*
         * Set the string describing this grouped relation to be used in EXPLAIN
-        * output of corresponding ForeignScan.
+        * output of corresponding ForeignScan.  Note that the decoration we add
+        * to the base relation name mustn't include any digits, or it'll confuse
+        * postgresExplainForeignScan.
         */
-       fpinfo->relation_name = makeStringInfo();
-       appendStringInfo(fpinfo->relation_name, "Aggregate on (%s)",
-                                        ofpinfo->relation_name->data);
+       fpinfo->relation_name = psprintf("Aggregate on (%s)",
+                                                                        ofpinfo->relation_name);
 
        return true;
 }
index bdefe0c40e2a13e788957b068dca10c5422afdce..ea052872c3e37533222636e72460fa5feabb5063 100644 (file)
@@ -87,11 +87,14 @@ typedef struct PgFdwRelationInfo
        int                     fetch_size;             /* fetch size for this remote table */
 
        /*
-        * Name of the relation while EXPLAINing ForeignScan. It is used for join
-        * relations but is set for all relations. For join relation, the name
-        * indicates which foreign tables are being joined and the join type used.
+        * Name of the relation, for use while EXPLAINing ForeignScan.  It is used
+        * for join and upper relations but is set for all relations.  For a base
+        * relation, this is really just the RT index as a string; we convert that
+        * while producing EXPLAIN output.  For join and upper relations, the name
+        * indicates which base foreign tables are included and the join type or
+        * aggregation type used.
         */
-       StringInfo      relation_name;
+       char       *relation_name;
 
        /* Join information */
        RelOptInfo *outerrel;