Re: Tab completion for ALTER INDEX|TABLE ALTER COLUMN SET STATISTICS

[Tatsuro Yamada]

On 2018/12/19 14:27, Michael Paquier wrote:

On Tue, Dec 04, 2018 at 02:31:51PM +0900, Tatsuro Yamada wrote:

  - For now, there is no column_number column on "\d index_name" result.
    So, if tab-completion suggested column_numbers, user can't match
    these easily.

Well, it depends how many columns an index definition has.  If that's
only a few then it is not really a problem.  However I agree that we
could add that in the output of \d for indexes just before the
definition to help with an easy match.  The columns showing up are
relkind-dependent so that's not an issue.  This would create some noise
in some regression tests.

I see.
Hopefully, I'll create new patch for adding column_number to \d for indexes.

So, we should better to vote to decide implementation of the tab-completion.

Which is better to use either column_names or column_numbers?
I'd like to hear opinions from others. :)

There has been a discussion in this area this week where the conclusion
is that we had better use column numbers rather than names arbitrarily
generated by the server for pg_dump:
https://postgr.es/m/CAARsnT3UQ4V=ydnw468w8rqhfyiy9mpn2r_c5ukbj97naap...@mail.gmail.com

So my take on the matter is to use column numbers, and show only those
with an expression as index attributes with no expressions cannot have
statistics.

Agreed.
I'll revise the patch replaced column_name with column_number.

Looking at the patches, fix_manual_of_alter_index.patch does not matter
much as the documentation of ALTER INDEX already mentions some
compatibilities with ALTER TABLE.

Hmm... I confused because these parameter are not same. Please see below:

====
  https://www.postgresql.org/docs/11/sql-altertable.html

    ALTER [ COLUMN ] column_name SET STATISTICS integer

  https://www.postgresql.org/docs/current/sql-alterindex.html

    ALTER [ COLUMN ] column_number SET STATISTICS integer
====

If we can use both parameter column_name and column_number, it would be better 
to
describe both on the documents.

+       /* ALTER TABLE ALTER [COLUMN] <foo> SET STATISTICS */
+       else if (HeadMatches("ALTER", "TABLE") && TailMatches("SET", 
"STATISTICS")){
+               /* We don't complete after "SET STATISTICS" */
+       }
Okay, this one makes sense taken independently as the current completion
is confusing.  Could you also do the same for ALTER INDEX?

Yep, I already did that for ALTER INDEX in 
tab_completion_alter_index_set_statistics.patch like this:

+   /* ALTER INDEX <name> ALTER COLUMN <colname> SET STATISTICS */
+   else if (HeadMatches("ALTER", "INDEX") && TailMatches("SET", "STATISTICS")){
+       /* We don't complete after "SET STATISTICS" */
+   }


Thanks,
Tatsuro Yamada