Re: adding tab completions

From: Justin Pryzby <pryzby(at)telsasoft(dot)com>
To: Arthur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>
Cc: Edmund Horner <ejrh00(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: adding tab completions
Date: 2018-06-09 23:42:12
Message-ID: 20180609234212.GT10885@telsasoft.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Thanks for review and comment.

On Tue, Jun 05, 2018 at 05:29:42PM +0300, Arthur Zakirov wrote:
> On Sun, Jun 03, 2018 at 10:39:22PM -0500, Justin Pryzby wrote:

> > > Also I think it could be good to list column names after parentheses,
> > > but I'm not sure if it easy to implement.
> > I tried this and nearly gave up, but see attached.
>
> After some thought now I think that this is not so good idea. The code
> doesn't look good too. It doesn't worth it and sorry for the distraction.

No problem.

> Moreover there is no such completion for example for the command (it shows
> only first column):
>
> CREATE INDEX ON test (

Noted (I misunderstood at first: you just mean there's precedent that column
names aren't completed, except maybe the first).

I can revise patch to not complete attributes in analyze; but, I think that
means that this will have to complete to table names:

postgres=# ANALYZE tt (a ,
alu_enodeb_201601 information_schema.
alu_enodeb_201602 jrn_pg_buffercache
...

.. since, without checking for matching parens, it has no idea whether to
complete with rels or atts. WDYT?

Note that HeadMatch functions have special behavior with matching parenthesis:
if previous char is an right parenthesis, then the "previous word" is taken to
be the entire parenthesized value. Maybe there's more that I don't know, but I
can't see how that can be easily applied here (by easily I mean without doing
something like making a temp copy of the buffer and inserting an "exploratory"
right parenthesis to see if TailMatches(prev_wd, ')') && prev_wd[0]=='(' or
similar).

An alternative is that only the first table is completed for vacuum/analyze.

CREATE INDEX should complete columns, too. It has the "ON" keyword (which
makes trying to parse less fragile).

> > - "SERVER", "INDEX", "LANGUAGE", "POLICY", "PUBLICATION", "RULE",
> > + "SERVER", "INDEX", "LANGUAGE", "POLICY", "PUBLICATION",
>
> Is this a typo? I think still there is a possibility to comment rules.

Not in PG11(b1) (note, that's a custom table)
postgres=# COMMENT ON RULE pg_settings_u IS 'asdf';
ERROR: syntax error at or near "IS"

I see I didn't include that description in my first mail.

Here's a description and possible commit message for the (still WIP) patch:

Update psql tab completion for commits:

Table completion for ANALYZE partitioned_table
3c3bb99330aa9b4c2f6258bfa0265d806bf365c3

Add parenthesized options syntax for ANALYZE.
854dd8cff523bc17972d34772b0e39ad3d6d46a4

Add VACUUM (DISABLE_PAGE_SKIPPING) for emergencies.
ede62e56fbe809baa1a7bc3873d82f12ffe7540b

Allow multiple tables to be specified in one VACUUM or ANALYZE command.
11d8d72c27a64ea4e30adce11cf6c4f3dd3e60db

Remove deprecated COMMENT ON RULE syntax
e8d016d81940e75c126aa52971b7903b7301002e

Add hash partitioning.
1aba8e651ac3e37e1d2d875842de1e0ed22a651e

Parameter toast_tuple_target
c2513365a0a85e77d3c21adb92fe12cfbe0d1897

Parenthesized explain (...)
d4382c4ae7ea1e272f4fee388aac8ff99421471a

Parameter toast_tuple_target controls TOAST for new rows
c2513365a0a85e77d3c21adb92fe12cfbe0d1897

no longer accepts VACUUM ANALYZE VERBOSE
921059bd66c7fb1230c705d3b1a65940800c4cbb

> > else if (HeadMatches1("EXPLAIN") && previous_words_count==2 && prev_wd[0]=='(' && ends_with(prev_wd, ')'))
>
> I think this condition can be replaced by:
>
> else if (TailMatches2("EXPLAIN", MatchAny) && ends_with(prev_wd, ')'))
>
> It looks better to me. Such condition is used for other commands and
> works the same way.
>
> The last point I've noticed, there is no VERBOSE entry after VACUUM FULL
> ANALYZE command anymore.

See commit 921059bd6, above (it's not 100% clear to me that's intended to
reject VACUUM ANALYZE VERBOSE and not just reject VACUUM VERBOSE ANALYZE
VERBOSE, but I tentatively assume it's intentional).

> I'm not sure how this patch should be commited. Can it be commited
> outside the commitfest? Otherwise add it to the next commitfest please
> in order not to forget it.

I've done https://commitfest.postgresql.org/18/1661/

Justin

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Gierth 2018-06-10 01:37:38 PostgreSQL vs SQL Standard
Previous Message Justin Pryzby 2018-06-09 23:20:58 Re: Postgres 11 release notes