Re: [PATCH] psql: Add tab-complete for optional view parameters

I noticed that on this commitfest entry
(https://commitfest.postgresql.org/44/4491/), the reviewers were
assigned by the patch author (presumably because they had previously
contributed to this thread). Unless these individuals know about that,
this is unlikely to work out. It's better to remove the reviewer
entries and let people sign up on their own. (You can nudge potential
candidates, of course.)

On 07.08.23 20:49, Christoph Heiss wrote:
>
> Hi all,
> sorry for the long delay.
>
> On Mon, Jan 09, 2023 at 04:32:09PM +0100, Jim Jones wrote:
>> However, an "ALTER TABLE <name> S<tab>" does not complete the open
>> parenthesis "(" from "SET (", as suggested in "ALTER VIEW <name> <tab>".
>>
>> postgres=# ALTER VIEW w SET
>> Display all 187 possibilities? (y or n)
>>
>> Is it intended to behave like this? If so, an "ALTER VIEW <name>
>> RES<tab>" does complete the open parenthesis -> "RESET (".
>
> On Sun, Jan 29, 2023 at 10:19:12AM +0000, Mikhail Gribkov wrote:
>> 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.
>> [..]
>> I think it may worth looking at "alter materialized view" completion
>> tree and making "alter view" the same way.
>
> Thank you both for reviewing/testing and the suggestions. Yeah,
> definitively, sounds very sensible.
>
> I've attached a new revision, rebased and addressing the above by
> aligning it with how "ALTER MATERIALIZED VIEW" works, such that "SET ("
> and "SET SCHEMA" won't compete anymore. So that should now work more
> like expected.
>
> postgres=# ALTER MATERIALIZED VIEW m
> ALTER COLUMN CLUSTER ON DEPENDS ON EXTENSION
> NO DEPENDS ON EXTENSION OWNER TO RENAME
> RESET ( SET
>
> postgres=# ALTER MATERIALIZED VIEW m SET
> ( ACCESS METHOD SCHEMA TABLESPACE
> WITHOUT CLUSTER
>
> postgres=# ALTER VIEW v
> ALTER COLUMN OWNER TO RENAME RESET ( SET
>
> postgres=# ALTER VIEW v SET
> ( SCHEMA
>
> postgres=# ALTER VIEW v SET (
> CHECK_OPTION SECURITY_BARRIER SECURITY_INVOKER
>
> On Fri, Jan 06, 2023 at 12:18:44PM +0000, Dean Rasheed wrote:
>> Hmm, I don't think we should be offering "check_option" as a tab
>> completion for CREATE VIEW at all, since that would encourage users to
>> use non-SQL-standard syntax, rather than CREATE VIEW ... WITH
>> [CASCADED|LOCAL] CHECK OPTION.
>
> Left that part in for now. I would argue that it is a well-documented
> combination and as such users would expect it to turn up in the
> tab-complete as well. OTOH not against removing it either, if there are
> others voicing the same opinion ..
>
> Thanks,
> Christoph