Re: BUG #16502: EXPLAIN JSON format adds extra quotes around index names

On Sun, Jun 21, 2020 at 4:29 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Julien Rouhaud <rjuju123@gmail.com> writes:
> > It turns out that in hypopg I totally missed that
> > explain_get_index_name_hook should take care of quoting the indexname.
> > People only want to know whether the hypothetical index is used or
> > not, so the lack of correct quoting didn't prevent that (the
> > hypothetical index name is automatically generated and includes its
> > oid).  The patch looks good to me, and +1 for backpatch!
>
> Ah, I'd wondered whether there might be hook users that "should" be
> doing quoting and were not.  So the impact on hypopg would be:
>
> 1. Non-text EXPLAIN formats are already properly quoted, and will
> remain so.

Correct.  Simple test using hypothetical index on a "T1" (id integer) table:

=# select * from hypopg_create_index('create index on "T1"(id)');
 indexrelid |      indexname
------------+---------------------
     576402 | <576402>btree_T1_id
(1 row)

Before:

=# explain (format yaml) select * from "T1" where id = 1;
              QUERY PLAN
---------------------------------------
 - Plan:                              +
     Node Type: "Index Only Scan"     +
     Parallel Aware: false            +
     Scan Direction: "Forward"        +
     Index Name: "<576402>btree_T1_id"+
     Relation Name: "T1"              +
     Alias: "T1"                      +
     Startup Cost: 0.04               +
     Total Cost: 8.06                 +
     Plan Rows: 1                     +
     Plan Width: 4                    +
     Index Cond: "(id = 1)"
(1 row)

After:

=# explain (format yaml)select * from "T1" where id = 1;
              QUERY PLAN
--------------------------------------
 - Plan:                             +
     Node Type: "Index Only Scan"    +
     Parallel Aware: false           +
     Scan Direction: "Forward"       +
     Index Name: "<16442>btree_T1_id"+
     Relation Name: "T1"             +
     Alias: "T1"                     +
     Startup Cost: 0.04              +
     Total Cost: 4.06                +
     Plan Rows: 1                    +
     Plan Width: 4                   +
     Index Cond: "(id = 1)"
(1 row)

> 2. Text-format EXPLAIN output will grow double-quotes around the
> hypothetical index names, which are not there at present (I believe,
> but didn't test it).  This shouldn't bother human users particularly,
> but conceivably it might break hypopg's regression tests?

Yes, the patch is working as expected.  Before:

=# explain select * from "T1" where id = 1;
                                     QUERY PLAN
-------------------------------------------------------------------------------------
 Index Only Scan using <576402>btree_T1_id on "T1"  (cost=0.04..8.06
rows=1 width=4)
   Index Cond: (id = 1)
(2 rows)

Patched:

=# explain select * from "T1" where id = 1;
                                      QUERY PLAN
--------------------------------------------------------------------------------------
 Index Only Scan using "<16442>btree_T1_id" on "T1"  (cost=0.04..4.06
rows=1 width=4)
   Index Cond: (id = 1)
(2 rows)

All of the regression tests except the one I added when the fix for
brin hypothetical indexes was applied are still working, as they're
looking for patterns like "Index.*<\d+>${amname}_${relname}" in the
textual explain output.  There are a few fixes not released yet, so
I'll update the broken brin regression test and publish a release soon
to make sure that the packages don't get broken when the new minor
versions are released.