Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1ibtWK-0003pA-Ct for pgsql-hackers@arkaria.postgresql.org; Mon, 02 Dec 2019 21:46:16 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1ibtWH-0001c6-Ug for pgsql-hackers@arkaria.postgresql.org; Mon, 02 Dec 2019 21:46:13 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1ibtWG-0001by-Pu for pgsql-hackers@lists.postgresql.org; Mon, 02 Dec 2019 21:46:12 +0000 Received: from sss.pgh.pa.us ([66.207.139.130]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1ibtVR-0006sD-EV for pgsql-hackers@lists.postgresql.org; Mon, 02 Dec 2019 21:46:11 +0000 Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1]) by sss.pgh.pa.us (8.14.4/8.14.4) with ESMTP id xB2LjICu008344; Mon, 2 Dec 2019 16:45:18 -0500 From: Tom Lane To: Etsuro Fujita cc: PostgreSQL Hackers Subject: Re: Bogus EXPLAIN results with column aliases for mismatched partitions 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> <23961.1575297703@sss.pgh.pa.us> Comments: In-reply-to Tom Lane message dated "Mon, 02 Dec 2019 09:41:43 -0500" MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0" Content-ID: <8307.1575323068.0@sss.pgh.pa.us> Date: Mon, 02 Dec 2019 16:45:18 -0500 Message-ID: <8343.1575323118@sss.pgh.pa.us> List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk ------- =_aaaaaaaaaa0 Content-Type: text/plain; charset="us-ascii" Content-ID: <8307.1575323068.1@sss.pgh.pa.us> Content-Transfer-Encoding: quoted-printable I wrote: > Etsuro Fujita writes: >> On Mon, Dec 2, 2019 at 6:34 AM Tom Lane 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 =3D=3D RTE_RELATION); objectname =3D get_rel_name(rte->relid); - if (es->verbose) + if (es->verbose || es->format !=3D EXPLAIN_FORMAT_TEXT) namespace =3D get_namespace_name(get_rel_namespace(rte->r= elid)); objecttag =3D "Relation Name"; break; (and likewise for function schema names, a bit further down)? regards, tom lane ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name="further-sync-postgres_fdw-with-regular-EXPLAIN.patch"; charset="us-ascii" Content-ID: <8307.1575323068.2@sss.pgh.pa.us> Content-Description: further-sync-postgres_fdw-with-regular-EXPLAIN.patch Content-Transfer-Encoding: quoted-printable diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/post= gres_fdw/expected/postgres_fdw.out index 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 =3D t2.b= ) INNER JOIN fprt1 t3 ON (t2.b =3D t3.a) WHERE t1.a % 25 =3D0 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.ftprt= 2_p1 t2)) INNER JOIN (public.ftprt1_p1 t3) + Relations: ((ftprt1_p1 t1) INNER JOIN (ftprt2_p1 t2)) INNE= R JOIN (ftprt1_p1 t3) -> Foreign Scan - Relations: ((public.ftprt1_p2 t1_1) INNER JOIN (public.ftp= rt2_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 =3D t2.b= ) INNER JOIN fprt1 t3 ON (t2.b =3D t3.a) WHERE t1.a % 25 =3D0 ORDER BY 1,2= ,3; @@ -8561,15 +8561,15 @@ SELECT t1.wr, t2.wr FROM (SELECT t1 wr, a FROM fpr= t1 t1 WHERE t1.a % 25 =3D 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 =3D t2.b AND t1.b =3D t2.a) q WHERE t1.a%25 =3D 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.ftpr= t2_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 =3D t2.b AND t1.b =3D t2.a) q WHERE t1.a%25 =3D 0 ORDER BY 1,2; @@ -8689,17 +8689,17 @@ SELECT a, sum(b), min(b), count(*) FROM pagg_tab G= ROUP 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/po= stgres_fdw.c index 3eb4e40..bdc21b3 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -2571,7 +2571,6 @@ postgresExplainForeignScan(ForeignScanState *node, E= xplainState *es) { int rti =3D strtol(ptr, &ptr, 10); RangeTblEntry *rte; - char *namespace; char *relname; char *refname; = @@ -2580,11 +2579,19 @@ postgresExplainForeignScan(ForeignScanState *node,= ExplainState *es) rte =3D rt_fetch(rti, es->rtable); Assert(rte->rtekind =3D=3D RTE_RELATION); /* This logic should agree with explain.c's ExplainTargetRel */ - namespace =3D get_namespace_name(get_rel_namespace(rte->relid)); relname =3D get_rel_name(rte->relid); - appendStringInfo(relations, "%s.%s", - quote_identifier(namespace), - quote_identifier(relname)); + if (es->verbose) + { + char *namespace; + + namespace =3D 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 =3D (char *) list_nth(es->rtable_names, rti - 1); if (refname =3D=3D NULL) refname =3D rte->eref->aliasname; ------- =_aaaaaaaaaa0--