Re: Bogus EXPLAIN results with column aliases for mismatched partitions

On Tue, Dec 3, 2019 at 6:45 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 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.

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