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

[Jim Jones]

Hi Christoph,




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


postgres=# ALTER VIEW w RESET (
CHECK_OPTION      SECURITY_BARRIER  SECURITY_INVOKER

postgres=# ALTER VIEW w SET (check_option =
CASCADED  LOCAL



postgres=# ALTER VIEW w SET
Display all 187 possibilities? (y or n)



Best,
Jim

On 09.12.22 11:31, Christoph Heiss wrote:

Thanks for the patch! I just tested it and I could reproduce the expected behaviour in these cases: ALTER COLUMN  OWNER TO      RENAME        RESET (       SET (         SET SCHEMA However, an "ALTER TABLE <name> S<tab>" does not complete the open parenthesis "(" from "SET (", as suggested in "ALTER VIEW <name> <tab>". Is it intended to behave like this? If so, an "ALTER VIEW <name> RES<tab>" does complete the open parenthesis -> "RESET (".

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");



I think it would be nice to include those missing words.

Also seems like SET and RESET don't get auto-completed for "ALTER VIEW <name>".

That is already contained in the patch:


     /* ALTER VIEW <name> */
     else if (Matches("ALTER", "VIEW", MatchAny))
         COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME",
-                      "SET SCHEMA");
+                      "SET SCHEMA", "SET (", "RESET (");

Thanks,
Christoph

@@ -2139,7 +2146,7 @@ psql_completion(const char *text, int start, int end)

smime.p7s
Description: S/MIME Cryptographic Signature