Re: Bogus EXPLAIN results with column aliases for mismatched partitions

From: Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Bogus EXPLAIN results with column aliases for mismatched partitions
Date: 2019-12-03 04:13:43
Message-ID: CAPmGK16+XskOXty=y+GP5B0MXs1o8gACq=qVbzbC3fRYrkByrA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Dec 3, 2019 at 6:45 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> I wrote:
> > Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com> writes:
> >> On Mon, Dec 2, 2019 at 6:34 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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.

Sorry, I misread the meaning.

> 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.

Thanks for the patch! (The patch didn't apply to HEAD cleanly,
though.) I like consistency, so +1 for the change.

> 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)?

Seems like another issue to me.

Best regards,
Etsuro Fujita

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2019-12-03 05:03:48 Re: Simplify passing of configure arguments to pg_config
Previous Message Michael Paquier 2019-12-03 04:03:01 Re: Failure in TAP tests of pg_ctl on Windows with parallel instance set