Re: Bogus EXPLAIN results with column aliases for mismatched partitions - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Bogus EXPLAIN results with column aliases for mismatched partitions
Date
Msg-id 4257.1575229116@sss.pgh.pa.us
Whole thread Raw
In response to Bogus EXPLAIN results with column aliases for mismatched partitions  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Bogus EXPLAIN results with column aliases for mismatched partitions
List pgsql-hackers
I wrote:
> * variant-a's diffs in expected/postgres_fdw.out indicate that
> postgres_fdw is doing something weird with the table aliases it selects to
> print in the "Relations:" output.  I think this is an independent bug ---
> and it surely *is* a bug, because the aliases printed by HEAD don't agree
> with the table aliases used for Vars of those relations.  But I haven't
> run it to ground yet.  (Notice that variant-b fixes those discrepancies in
> the opposite direction...)

I checked that, and indeed postgres_fdw is doing something randomly
different from what ruleutils does.  In set_rtable_names(), the
first priority is rte->alias->aliasname, and if that's not set
then (for a RELATION RTE) you get the result of get_rel_name().
postgres_fdw is taking rte->eref->aliasname as being the alias,
which is usually the same string, but "usually" doesn't cut it.
So we should make it look at rte->alias instead.

Now, there is another thing that set_rtable_names() is doing that
postgres_fdw doesn't do, which is to unique-ify the chosen alias
names by adding "_NN" if the querytree contains multiple uses of
the same table alias.  I don't see any practical way for postgres_fdw
to match that behavior, since it lacks global information about the
query.  If we wanted to fix it, what we'd likely need to do is
postpone creation of the relation_name strings until EXPLAIN,
providing some way for postgres_fdw to ask ruleutils.c what alias
it'd assigned to a particular RTE.  This seems like it wouldn't be
terribly painful for base relations but it'd be a mess for joins
and aggregates, so I'm not eager to do something like that.
In practice the presence or absence of "_NN" might not be too
confusing --- it's certainly not as bad as the inconsistency being
shown now.

In short then I propose the attached fix for this issue.

            regards, tom lane

diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 14180ee..5f7aa31 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -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
NULLSLAST 
 (4 rows)

@@ -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;
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index fa14296..1bed907 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -576,7 +576,6 @@ postgresGetForeignRelSize(PlannerInfo *root,
     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
@@ -727,13 +726,12 @@ postgresGetForeignRelSize(PlannerInfo *root,
     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)
+    if (rte->alias && strcmp(rte->alias->aliasname, relname) != 0)
         appendStringInfo(fpinfo->relation_name, " %s",
-                         quote_identifier(rte->eref->aliasname));
+                         quote_identifier(rte->alias->aliasname));

     /* No outer and inner relations. */
     fpinfo->make_outerrel_subquery = false;

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: surprisingly expensive join planning query
Next
From: Tom Lane
Date:
Subject: Re: Bogus EXPLAIN results with column aliases for mismatched partitions