Обсуждение: display of variables in EXPLAIN VERBOSE

Hi,

ISTM show_plan_tlist()'s rule of whether to the show range table prefix
with displayed variables contradicts the description of the VERBOSE option
in EXPLAIN documentation, which is as follows:

=======
VERBOSE

Display additional information regarding the plan. Specifically, include
the output column list for each node in the plan tree, schema-qualify
table and function names, always label variables in expressions with their
range table alias, and always print the name of each trigger for which
statistics are displayed. This parameter defaults to FALSE.
=======

Specifically, the current behavior contradicts the part of the sentence
that says "always label variables in expressions with their range table
alias".  See this example:

create table foo (a int);
create table foo1 () inherits (foo);

-- "a" is not labeled here
explain verbose select * from only foo order by 1;
                           QUERY PLAN
────────────────────────────────────────────────────────────────
 Sort  (cost=0.01..0.02 rows=1 width=4)
   Output: a
   Sort Key: foo.a
   ->  Seq Scan on public.foo  (cost=0.00..0.00 rows=1 width=4)
         Output: a
(5 rows)

-- it's labeled in this case
explain verbose select * from foo order by 1;
                                QUERY PLAN
───────────────────────────────────────────────────────────────────────────
 Sort  (cost=192.60..198.98 rows=2551 width=4)
   Output: foo.a
   Sort Key: foo.a
   ->  Append  (cost=0.00..48.26 rows=2551 width=4)
         ->  Seq Scan on public.foo  (cost=0.00..0.00 rows=1 width=4)
               Output: foo.a
         ->  Seq Scan on public.foo1  (cost=0.00..35.50 rows=2550 width=4)
               Output: foo1.a
(8 rows)

Seeing that "Sort Key" is always displayed with the range table alias, I
checked explain.c to see why the discrepancy exists and it seems that
show_plan_tlist() (and show_tablesample()) use the following condition for
whether or not to use the range table prefix:

    useprefix = list_length(es->rtable) > 1;

whereas other functions, including show_sort_group_keys() that prints the
"Sort Key", use the following condition:

    useprefix = (list_length(es->rtable) > 1 || es->verbose);

I can think of two ways we could do:

1. Change show_plan_tlist() and show_tablesample() to use the same rule as
others

2. Change other functions to use the same rule as show_plan_tlist(), also
updating the documentation to note the exceptional case when column names
are not prefixed

Thoughts?

Thanks,
Amit

On Mon, 22 Apr 2019 at 19:49, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
> Seeing that "Sort Key" is always displayed with the range table alias, I
> checked explain.c to see why the discrepancy exists and it seems that
> show_plan_tlist() (and show_tablesample()) use the following condition for
> whether or not to use the range table prefix:
>
>     useprefix = list_length(es->rtable) > 1;
>
> whereas other functions, including show_sort_group_keys() that prints the
> "Sort Key", use the following condition:
>
>     useprefix = (list_length(es->rtable) > 1 || es->verbose);
>
> I can think of two ways we could do:
>
> 1. Change show_plan_tlist() and show_tablesample() to use the same rule as
> others
>
> 2. Change other functions to use the same rule as show_plan_tlist(), also
> updating the documentation to note the exceptional case when column names
> are not prefixed

I'd vote to make the code match the documentation, but probably
implement it by adding a new field to ExplainState and just calculate
what to do once in ExplainQuery() instead of calculating what to do in
various random places.

I don't think we should backpatch this change, likely it would be
better to keep the explain output as stable as possible in the back
branches, so that might mean a documentation tweak should be done for
them.

-- 
 David Rowley                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

David Rowley <david.rowley@2ndquadrant.com> writes:
> I'd vote to make the code match the documentation, but probably
> implement it by adding a new field to ExplainState and just calculate
> what to do once in ExplainQuery() instead of calculating what to do in
> various random places.

Yeah, this is none too consistent:

$ grep -n 'useprefix =' explain.c
2081:   useprefix = list_length(es->rtable) > 1;
2151:   useprefix = (IsA(planstate->plan, SubqueryScan) ||es->verbose);
2165:   useprefix = (list_length(es->rtable) > 1 || es->verbose);
2238:   useprefix = (list_length(es->rtable) > 1 || es->verbose);
2377:   useprefix = (list_length(es->rtable) > 1 || es->verbose);
2485:   useprefix = list_length(es->rtable) > 1;

If we're going to mess with this, I'd also suggest that we not depend on
list_length(es->rtable) per se, as that counts RTEs that may have nothing
to do with the plan.  For instance, I've never been very happy about
this behavior:

regression=# create table tt (f1 int, f2 int);
CREATE TABLE
regression=# explain verbose select * from tt;
                         QUERY PLAN                          
-------------------------------------------------------------
 Seq Scan on public.tt  (cost=0.00..32.60 rows=2260 width=8)
   Output: f1, f2
(2 rows)

regression=# create view vv as select * from tt;
CREATE VIEW
regression=# explain verbose select * from vv;
                         QUERY PLAN                          
-------------------------------------------------------------
 Seq Scan on public.tt  (cost=0.00..32.60 rows=2260 width=8)
   Output: tt.f1, tt.f2
(2 rows)

The reason for the difference is the presence of the view's RTE
in the plan, but why should that affect the printout?  Maybe we
could make it depend on the number of RTE names assigned by
select_rtable_names_for_explain, instead.

BTW, now that I look at this, I think the reason why I didn't make
tlist printouts pay attention to VERBOSE for this purpose is that
you don't get them at all if not verbose:

regression=# explain select * from tt;
                      QUERY PLAN                      
------------------------------------------------------
 Seq Scan on tt  (cost=0.00..32.60 rows=2260 width=8)
(1 row)

So if we were to be rigidly consistent with this point of the docs,
there would be no way to see a tlist without variable qualification,
which doesn't really seem that nice.

Alternatively, we could just leave this as-is.  I do not think the
quoted doc paragraph was ever meant as an exact specification
of what EXPLAIN VERBOSE does, nor do I believe that making it so
would be helpful.

            regards, tom lane

On 2019/04/23 0:58, Tom Lane wrote:
> BTW, now that I look at this, I think the reason why I didn't make
> tlist printouts pay attention to VERBOSE for this purpose is that
> you don't get them at all if not verbose:
> 
> regression=# explain select * from tt;
>                       QUERY PLAN                      
> ------------------------------------------------------
>  Seq Scan on tt  (cost=0.00..32.60 rows=2260 width=8)
> (1 row)
> 
> So if we were to be rigidly consistent with this point of the docs,
> there would be no way to see a tlist without variable qualification,
> which doesn't really seem that nice.

Hmm yes.  Variables in sort keys, quals, etc., which are shown without
VERBOSE, are qualified only if VERBOSE is specified.  Variables in the
targetlists that are shown only in the VERBOSE output may be displayed
without qualifications, which looks a bit inconsistent.

explain (verbose, costs off) select * from foo where a > 0 order by 1;
          QUERY PLAN
──────────────────────────────
 Sort
   Output: a
   Sort Key: foo.a
   ->  Seq Scan on public.foo
         Output: a
         Filter: (foo.a > 0)
(6 rows)

Maybe, targetlist variables should *always* be qualified given that they
are considered VERBOSE information to begin with?

Thanks,
Amit