public inbox for [email protected]  

help / color / mirror / Atom feed

From: Mikhail Gribkov <[email protected]>
To: [email protected]
Cc: Christoph Heiss <[email protected]>
Subject: Re: [PATCH] psql: Add tab-complete for optional view parameters
Date: Sun, 29 Jan 2023 10:19:12 +0000
Message-ID: <167498755212.1114.17909291598039410948.pgcf@coridan.postgresql.org> (raw)
In-Reply-To: <CA+hUKGLiY1e+1=pB7hXJOyGj1dJOfgde+HmiSnv3gDKayUFJMA@mail.gmail.com>
References: <[email protected]>
	<CA+hUKGLiY1e+1=pB7hXJOyGj1dJOfgde+HmiSnv3gDKayUFJMA@mail.gmail.com>

The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:       tested, passed
Spec compliant:           tested, failed
Documentation:            tested, passed

Hi Christoph,

The patch have a potential, although I have to agree with Jim Jones, it obviously have a problem with "alter view <name> set<tab>" handling.

First of all user can notice, that SET and RESET alternatives are proposed in an absolutely equivalent way:
postgres=# alter view VVV <tab>
ALTER COLUMN  OWNER TO      RENAME        RESET (       SET (         SET SCHEMA

But completion of a parentheses differs fore these alternatives:

postgres=# alter view VVV reset<tab> -> completes with "<space>(" -> <tab>
CHECK_OPTION      SECURITY_BARRIER  SECURITY_INVOKER

postgres=# alter view VVV set<tab> -> completes with a single spase -> <tab>
Display all 188 possibilities? (y or n)
(and these 188 possibilities do not contain "(")

The probmen is obviously in the newly added second line of the following clause:
		COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME",
					  "SET SCHEMA", "SET (", "RESET (");

"set schema" and "set (" alternatives are competing, while completion of the common part "set<space>" leads to a string composition which does not have the check branch (Matches("ALTER", "VIEW", MatchAny, "SET")).

I think it may worth looking at "alter materialized view"  completion tree and making "alter view" the same way.

The new status of this patch is: Waiting on Author

view thread (5+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: [PATCH] psql: Add tab-complete for optional view parameters
  In-Reply-To: <167498755212.1114.17909291598039410948.pgcf@coridan.postgresql.org>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox