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

From: Christoph Heiss <[email-masked]>
Date: Mon, 7 Aug 2023 20:49:04 +0200

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
>From 3663eb0b5008d632972d4b66a105fc08cfff13fb Mon Sep 17 00:00:00 2001
From: Christoph Heiss <[email-masked]>
Date: Mon, 7 Aug 2023 20:37:19 +0200
Subject: [PATCH v3] psql: Add tab-complete for optional view parameters

This adds them in the same matter as it works for storage parameters of
tables.

Signed-off-by: Christoph Heiss <[email-masked]>
---
 src/bin/psql/tab-complete.c | 36 +++++++++++++++++++++++++++++++-----
 1 file changed, 31 insertions(+), 5 deletions(-)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 779fdc90cb..83ec1508bb 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1329,6 +1329,13 @@ static const char *const table_storage_parameters[] = {
 	NULL
 };
 
+/* Optional parameters for CREATE VIEW and ALTER VIEW */
+static const char *const view_optional_parameters[] = {
+	"check_option",
+	"security_barrier",
+	"security_invoker",
+	NULL
+};
 
 /* Forward declaration of functions */
 static char **psql_completion(const char *text, int start, int end);
@@ -2216,8 +2223,7 @@ psql_completion(const char *text, int start, int end)
 		COMPLETE_WITH("TO");
 	/* ALTER VIEW <name> */
 	else if (Matches("ALTER", "VIEW", MatchAny))
-		COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME",
-					  "SET SCHEMA");
+		COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME", "RESET (", "SET");
 	/* ALTER VIEW xxx RENAME */
 	else if (Matches("ALTER", "VIEW", MatchAny, "RENAME"))
 		COMPLETE_WITH_ATTR_PLUS(prev2_wd, "COLUMN", "TO");
@@ -2233,6 +2239,16 @@ psql_completion(const char *text, int start, int end)
 	/* ALTER VIEW xxx RENAME COLUMN yyy */
 	else if (Matches("ALTER", "VIEW", MatchAny, "RENAME", "COLUMN", MatchAnyExcept("TO")))
 		COMPLETE_WITH("TO");
+	/* ALTER VIEW xxx SET ( yyy [= zzz] ) */
+	else if (Matches("ALTER", "VIEW", MatchAny, "SET"))
+		COMPLETE_WITH("(", "SCHEMA");
+	/* ALTER VIEW xxx SET|RESET ( yyy [= zzz] ) */
+	else if (Matches("ALTER", "VIEW", MatchAny, "SET|RESET", "("))
+		COMPLETE_WITH_LIST(view_optional_parameters);
+	else if (Matches("ALTER", "VIEW", MatchAny, "SET", "(", "check_option", "="))
+		COMPLETE_WITH("local", "cascaded");
+	else if (Matches("ALTER", "VIEW", MatchAny, "SET", "(", "security_barrier|security_invoker", "="))
+		COMPLETE_WITH("true", "false");
 
 	/* ALTER MATERIALIZED VIEW <name> */
 	else if (Matches("ALTER", "MATERIALIZED", "VIEW", MatchAny))
@@ -3525,13 +3541,23 @@ psql_completion(const char *text, int start, int end)
 	}
 
 /* CREATE VIEW --- is allowed inside CREATE SCHEMA, so use TailMatches */
-	/* Complete CREATE [ OR REPLACE ] VIEW <name> with AS */
+	/* Complete CREATE [ OR REPLACE ] VIEW <name> with AS or WITH ( */
 	else if (TailMatches("CREATE", "VIEW", MatchAny) ||
 			 TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny))
+		COMPLETE_WITH("AS", "WITH (");
+	/* Complete CREATE [ OR REPLACE ] VIEW <name> WITH ( with supported options */
+	else if (TailMatches("CREATE", "VIEW", MatchAny, "WITH", "(") ||
+			 TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "WITH", "("))
+		COMPLETE_WITH_LIST(view_optional_parameters);
+	/* Complete CREATE [ OR REPLACE ] VIEW <name> WITH ( ... ) with "AS" */
+	else if (TailMatches("CREATE", "VIEW", MatchAny, "WITH", "(*)") ||
+			 TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "WITH", "(*)"))
 		COMPLETE_WITH("AS");
-	/* Complete "CREATE [ OR REPLACE ] VIEW <sth> AS with "SELECT" */
+	/* Complete "CREATE [ OR REPLACE ] VIEW <sth> [ WITH ( ... ) ] AS with "SELECT" */
 	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"))
 		COMPLETE_WITH("SELECT");
 
 /* CREATE MATERIALIZED VIEW */
-- 
2.41.0