On Fri, Aug 11, 2023 at 12:48:17PM -0700, David Zhang wrote:
>
> Applied v3 patch to master and verified it with below commands,
Thanks for testing!

> [..]
>
> For below changes,
>
>      else if (TailMatches("CREATE", "VIEW", MatchAny, "AS") ||
> -             TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny,
> "AS"))
> +             TailMatches("CREATE", "VIEW", MatchAny, "WITH", "(*)", "AS")
> ||
> +             TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "AS")
> ||
> +             TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny,
> "WITH", "(*)", "AS"))
>
> it would be great to switch the order of the 3rd and the 4th line to make a
> better match for "CREATE" and "CREATE OR REPLACE" .

I don't see how it would effect matching in any way - or am I
overlooking something here?

postgres=# CREATE VIEW v <tab>
AS      WITH (

postgres=# CREATE VIEW v AS <tab>
.. autocompletes with "SELECT"

postgres=# CREATE VIEW v WITH ( security_invoker = true ) <tab>
.. autocompletes with "AS" and so on

And the same for CREATE OR REPLACE VIEW.

Can you provide an example case that would benefit from that?

>
> Since it supports <tab> in the middle for below case,
> postgres=# alter view v set ( security_<tab>
> security_barrier  security_invoker
>
> and during view reset it can also provide all the options list,
> postgres=# alter view v reset (
> CHECK_OPTION      SECURITY_BARRIER  SECURITY_INVOKER
>
> but not sure if it is a good idea or possible to autocomplete the reset
> options after seeing one of the options showing up with "," for example,
> postgres=# alter view v reset ( CHECK_OPTION, <tab>
> SECURITY_BARRIER  SECURITY_INVOKER

I'd rather not add this and leave it as-is. It doesn't add any real
value - how often does this case really come up, especially with ALTER
VIEW only having three options?

Thanks,
Christoph


Reply via email to