From: Tom Lane <tgl@sss.pgh.pa.us>
To: Etsuro Fujita <etsuro.fujita@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Bogus EXPLAIN results with column aliases for mismatched partitions
Date: Mon, 02 Dec 2019 16:45:18 -0500
Message-ID: <8343.1575323118@sss.pgh.pa.us> (raw)
In-Reply-To: <23961.1575297703@sss.pgh.pa.us>
References: <12424.1575168015@sss.pgh.pa.us>
<4257.1575229116@sss.pgh.pa.us>
<10395.1575236078@sss.pgh.pa.us>
<CAPmGK15KSu5DkFjRJ96PHVmwti9FSvbMnM2_d2=GXx4iw_jrxw@mail.gmail.com>
<23961.1575297703@sss.pgh.pa.us>
I wrote:
> Etsuro Fujita <etsuro.fujita@gmail.com> writes:
>> On Mon, Dec 2, 2019 at 6:34 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> BTW, the existing code always schema-qualifies the relation names,
>>> on the rather lame grounds that it's producing the string without
>>> knowing whether EXPLAIN VERBOSE will be specified. In this code,
>>> the verbose flag is available so it would be trivial to make the
>>> output conform to EXPLAIN's normal policy. I didn't change that
>>> here because there'd be a bunch more test output diffs of no
>>> intellectual interest. Should we change it, or leave well enough
>>> alone?
>> I think it would be better to keep that as-is because otherwise, in
>> case of a foreign join or aggregate, EXPLAIN without the VERBOSE
>> option won't show any information about foreign tables involved in
>> that foreign join or aggregate, which isn't useful for users.
> No, I'm just talking about dropping the schema-qualification of table
> names when !es->verbose, not removing the Relations: output altogether.
> That would be more consistent with the rest of EXPLAIN's output.
Concretely, I'm thinking of the attached (on top of the other patch,
which I just pushed). This agrees exactly with what ExplainTargetRel
does for regular scans.
One could make an argument that we should schema-qualify, regardless
of the "verbose" flag, if the output format isn't EXPLAIN_FORMAT_TEXT.
That would reduce the amount of variability that plan analysis tools
have to cope with. However, ExplainTargetRel itself doesn't provide
the schema in non-verbose mode. Maybe it should, ie we should change
it like
case T_ModifyTable:
/* Assert it's on a real relation */
Assert(rte->rtekind == RTE_RELATION);
objectname = get_rel_name(rte->relid);
- if (es->verbose)
+ if (es->verbose || es->format != EXPLAIN_FORMAT_TEXT)
namespace = get_namespace_name(get_rel_namespace(rte->relid));
objecttag = "Relation Name";
break;
(and likewise for function schema names, a bit further down)?
regards, tom lane
Attachments:
[text/x-diff] further-sync-postgres_fdw-with-regular-EXPLAIN.patch (5.2K, ../8343.1575323118@sss.pgh.pa.us/2-further-sync-postgres_fdw-with-regular-EXPLAIN.patch)
download | inline diff:diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.outindex d276545..95ed602 100644--- a/contrib/postgres_fdw/expected/postgres_fdw.out+++ b/contrib/postgres_fdw/expected/postgres_fdw.out@@ -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)+ Relations: ((ftprt1_p1 t1) INNER JOIN (ftprt2_p1 t2)) INNER JOIN (ftprt1_p1 t3)
-> Foreign Scan
- Relations: ((public.ftprt1_p2 t1_1) INNER JOIN (public.ftprt2_p2 t2_1)) INNER JOIN (public.ftprt1_p2 t3_1)+ Relations: ((ftprt1_p2 t1_1) INNER JOIN (ftprt2_p2 t2_1)) INNER JOIN (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;
@@ -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)+ Relations: (ftprt1_p1 t1) INNER JOIN (ftprt2_p1 t2)
-> Foreign Scan
- Relations: (public.ftprt1_p2 t1_1) INNER JOIN (public.ftprt2_p2 t2_1)+ Relations: (ftprt1_p2 t1_1) INNER JOIN (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)+ Relations: Aggregate on (fpagg_tab_p1)
-> Foreign Scan
- Relations: Aggregate on (public.fpagg_tab_p2)+ Relations: Aggregate on (fpagg_tab_p2)
-> Foreign Scan
- Relations: Aggregate on (public.fpagg_tab_p3)+ Relations: Aggregate on (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.cindex 3eb4e40..bdc21b3 100644--- a/contrib/postgres_fdw/postgres_fdw.c+++ b/contrib/postgres_fdw/postgres_fdw.c@@ -2571,7 +2571,6 @@ postgresExplainForeignScan(ForeignScanState *node, ExplainState *es)
{
int rti = strtol(ptr, &ptr, 10);
RangeTblEntry *rte;
- char *namespace;
char *relname;
char *refname;
@@ -2580,11 +2579,19 @@ postgresExplainForeignScan(ForeignScanState *node, ExplainState *es)
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));+ if (es->verbose)+ {+ char *namespace;++ namespace = get_namespace_name(get_rel_namespace(rte->relid));+ appendStringInfo(relations, "%s.%s",+ quote_identifier(namespace),+ quote_identifier(relname));+ }+ else+ appendStringInfo(relations, "%s",+ quote_identifier(relname));
refname = (char *) list_nth(es->rtable_names, rti - 1);
if (refname == NULL)
refname = rte->eref->aliasname;
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, etsuro.fujita@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: Bogus EXPLAIN results with column aliases for mismatched partitions
In-Reply-To: <8343.1575323118@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox