From: | Jim Jones <jim(dot)jones(at)uni-muenster(dot)de> |
---|---|
To: | Christoph Heiss <christoph(at)c8h4(dot)io>, Melih Mutlu <m(dot)melihmutlu(at)gmail(dot)com> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [PATCH] psql: Add tab-complete for optional view parameters |
Date: | 2023-01-09 15:32:09 |
Message-ID: | e644cb86-a5a9-50b6-d2e3-ab4e6a0b99c6@uni-muenster.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi Christoph,
Thanks for the patch! I just tested it and I could reproduce the
expected behaviour in these cases:
postgres=# CREATE VIEW w
AS WITH (
postgres=# CREATE OR REPLACE VIEW w
AS WITH (
postgres=# CREATE VIEW w WITH (
CHECK_OPTION SECURITY_BARRIER SECURITY_INVOKER
postgres=# CREATE OR REPLACE VIEW w WITH (
CHECK_OPTION SECURITY_BARRIER SECURITY_INVOKER
postgres=# ALTER VIEW w
ALTER COLUMN OWNER TO RENAME RESET ( SET (
SET SCHEMA
postgres=# ALTER VIEW w RESET (
CHECK_OPTION SECURITY_BARRIER SECURITY_INVOKER
postgres=# ALTER VIEW w SET (check_option =
CASCADED LOCAL
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 (".
Best,
Jim
On 09.12.22 11:31, Christoph Heiss wrote:
> Thanks for the review!
>
> On 12/8/22 12:19, Melih Mutlu wrote:
>> Hi Christoph,
>>
>> I just took a quick look at your patch.
>> Some suggestions:
>>
>> + else if (Matches("ALTER", "VIEW", MatchAny, "SET", "("))
>> + COMPLETE_WITH_LIST(view_optional_parameters);
>> + /* ALTER VIEW xxx RESET ( yyy , ... ) */
>> + else if (Matches("ALTER", "VIEW", MatchAny, "RESET", "("))
>> + COMPLETE_WITH_LIST(view_optional_parameters);
>>
>>
>> What about combining these two cases into one like Matches("ALTER",
>> "VIEW", MatchAny, "SET|RESET", "(") ?
> Good thinking, incorporated that into v2.
> While at it, I also added completion for the values of the SET
> parameters, since that is useful as well.
>
>>
>> /* ALTER VIEW <name> */
>> else if (Matches("ALTER", "VIEW", MatchAny))
>> COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME",
>> "SET SCHEMA");
>>
>>
>> Also seems like SET and RESET don't get auto-completed for "ALTER
>> VIEW <name>".
>> I think it would be nice to include those missing words.
> That is already contained in the patch:
>
> @@ -2139,7 +2146,7 @@ psql_completion(const char *text, int start, int
> end)
> /* ALTER VIEW <name> */
> else if (Matches("ALTER", "VIEW", MatchAny))
> COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME",
> - "SET SCHEMA");
> + "SET SCHEMA", "SET (", "RESET (");
>
> Thanks,
> Christoph
From | Date | Subject | |
---|---|---|---|
Next Message | Laurenz Albe | 2023-01-09 15:40:10 | Re: Postgres Partitions Limitations (5.11.2.3) |
Previous Message | Tom Lane | 2023-01-09 15:26:23 | Re: [PATCH] random_normal function |