Feedback-ID: i1ff147bf:Fastmail
From: =?utf-8?Q?Dagfinn_Ilmari_Manns=C3=A5ker?= <ilmari@ilmari.org>
To: Michael Paquier <michael@paquier.xyz>
Cc: Suraj Khamkar <khamkarsuraj.b@gmail.com>,
  pgsql-hackers@lists.postgresql.org
Subject: Re: Tab completion for CREATE SCHEMAAUTHORIZATION
References: <874kc1gdxc.fsf@wibble.ilmari.org>
	<CA+U=F9gP3RWQEVasStdpVw0_bOXeOLxQYspwkR+76BhYprXToQ@mail.gmail.com>
	<87im0efqhp.fsf@wibble.ilmari.org> <YRMk378EUuVmJ2Fq@paquier.xyz>
	<Yalwm1pc6X+GVxWn@paquier.xyz> <87ildyqwks.fsf@wibble.ilmari.org>
	<ZDoGocfchN9KFxFC@paquier.xyz> <ZFBw99aDs5AczF6v@paquier.xyz>
	<875y9buhys.fsf@wibble.ilmari.org> <87354fudqy.fsf@wibble.ilmari.org>
	<ZFXCY0s1D1KtJi+5@paquier.xyz>
Date: Mon, 08 May 2023 17:36:27 +0100
In-Reply-To: <ZFXCY0s1D1KtJi+5@paquier.xyz> (Michael Paquier's message of
	"Sat, 6 May 2023 11:58:43 +0900")
Message-ID: <87ttwmu6es.fsf@wibble.ilmari.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="=-=-="
Archived-At: 
 <https://www.postgresql.org/message-id/87ttwmu6es.fsf%40wibble.ilmari.org>
Precedence: bulk

--=-=-=
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit

Michael Paquier <michael@paquier.xyz> writes:

> On Tue, May 02, 2023 at 01:19:49PM +0100, Dagfinn Ilmari Mannsåker wrote:
>> Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> writes:
>>> This is for completing the word CREATE itself after CREATE SCHEMA
>>> [[<name>] AUTHORIZATION] <name>.  The things that can come after that
>>> are already handled generically earlier in the function:
>>>
>>> /* CREATE */
>>>     /* complete with something you can create */
>>>     else if (TailMatches("CREATE"))
>>>         matches = rl_completion_matches(text, create_command_generator);
>>>
>>> create_command_generator uses the words_after_create array, which lists
>>> all the things that can be created.
>
> You are right.  I have completely forgotten that this code path would
> append everything that supports CREATE for a CREATE SCHEMA command :)
>
>> But, looking closer at the docs, only tables, views, indexes, sequences
>> and triggers can be created as part of a CREATE SCHEMA statement. Maybe
>> we should add a HeadMatches("CREATE", "SCHEMA") exception in the above?
>
> Yes, it looks like we are going to need an exception and append only
> the keywords that are supported, or we will end up recommending mostly
> things that are not accepted by the parser.

Here's an updated v3 patch with that.  While adding that, I noticed that
CREATE UNLOGGED only tab-completes TABLE and MATERIALIZED VIEW, not
SEQUENCE, so I added that (and removed MATERIALIZED VIEW when part of
CREATE SCHEMA).

- ilmari


--=-=-=
Content-Type: text/x-diff
Content-Disposition: inline;
 filename=v3-0001-Add-tab-completion-for-CREATE-SCHEMA.patch

From 31856bf5397253da76cbce9ccb590855a44da30d Mon Sep 17 00:00:00 2001
From: tanghy <tanghy.fnst@fujitsu.com>
Date: Mon, 9 Aug 2021 18:47:12 +0100
Subject: [PATCH v3] Add tab completion for CREATE SCHEMA

 - AUTHORIZATION both in addition to and after a schema name
 - list of owner roles after AUTHORIZATION
 - CREATE and GRANT after any otherwise-complete command
 - Only the allowed object types after CREATE

Also adjust complation after CREATE UNLOGGED:

 - Add SEQUENCE
 - Don't suggest MATERIALIZED VIEW in CREATE TABLE
---
 src/bin/psql/tab-complete.c | 40 ++++++++++++++++++++++++++++++-------
 1 file changed, 33 insertions(+), 7 deletions(-)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index bd04244969..66b3f00c1b 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1031,9 +1031,13 @@ static const SchemaQuery Query_for_trigger_of_table = {
 "   FROM pg_catalog.pg_roles "\
 "  WHERE rolname LIKE '%s'"
 
+/* add these to Query_for_list_of_roles in OWNER contexts */
+#define Keywords_for_list_of_owner_roles \
+"CURRENT_ROLE", "CURRENT_USER", "SESSION_USER"
+
 /* add these to Query_for_list_of_roles in GRANT contexts */
 #define Keywords_for_list_of_grant_roles \
-"PUBLIC", "CURRENT_ROLE", "CURRENT_USER", "SESSION_USER"
+Keywords_for_list_of_owner_roles, "PUBLIC"
 
 #define Query_for_all_table_constraints \
 "SELECT conname "\
@@ -1785,7 +1789,13 @@ psql_completion(const char *text, int start, int end)
 /* CREATE */
 	/* complete with something you can create */
 	else if (TailMatches("CREATE"))
-		matches = rl_completion_matches(text, create_command_generator);
+		/* only some object types can be created as part of CREATE SCHEMA */
+		if (HeadMatches("CREATE", "SCHEMA"))
+			COMPLETE_WITH("TABLE", "VIEW", "INDEX", "SEQUENCE", "TRIGGER",
+						  /* for INDEX and TABLE/SEQUENCE, respectively */
+						  "UNIQUE", "UNLOGGED");
+		else
+			matches = rl_completion_matches(text, create_command_generator);
 
 	/* complete with something you can create or replace */
 	else if (TailMatches("CREATE", "OR", "REPLACE"))
@@ -3154,6 +3164,20 @@ psql_completion(const char *text, int start, int end)
 	else if (TailMatches("AS", "ON", "SELECT|UPDATE|INSERT|DELETE", "TO"))
 		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables);
 
+/* CREATE SCHEMA [ <name> ] [ AUTHORIZATION ] */
+	else if (Matches("CREATE", "SCHEMA"))
+		COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_schemas,
+								 "AUTHORIZATION");
+	else if (Matches("CREATE", "SCHEMA", "AUTHORIZATION") ||
+			 Matches("CREATE", "SCHEMA", MatchAny, "AUTHORIZATION"))
+		COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_roles,
+								 Keywords_for_list_of_owner_roles);
+	else if (Matches("CREATE", "SCHEMA", "AUTHORIZATION", MatchAny) ||
+			 Matches("CREATE", "SCHEMA", MatchAny, "AUTHORIZATION", MatchAny))
+		COMPLETE_WITH("CREATE", "GRANT");
+	else if (Matches("CREATE", "SCHEMA", MatchAny))
+		COMPLETE_WITH("AUTHORIZATION", "CREATE", "GRANT");
+
 /* CREATE SEQUENCE --- is allowed inside CREATE SCHEMA, so use TailMatches */
 	else if (TailMatches("CREATE", "SEQUENCE", MatchAny) ||
 			 TailMatches("CREATE", "TEMP|TEMPORARY", "SEQUENCE", MatchAny))
@@ -3185,9 +3209,13 @@ psql_completion(const char *text, int start, int end)
 	/* Complete "CREATE TEMP/TEMPORARY" with the possible temp objects */
 	else if (TailMatches("CREATE", "TEMP|TEMPORARY"))
 		COMPLETE_WITH("SEQUENCE", "TABLE", "VIEW");
-	/* Complete "CREATE UNLOGGED" with TABLE or MATVIEW */
+	/* Complete "CREATE UNLOGGED" with TABLE, SEQUENCE or MATVIEW */
 	else if (TailMatches("CREATE", "UNLOGGED"))
-		COMPLETE_WITH("TABLE", "MATERIALIZED VIEW");
+		/* but not MATVIEW in CREATE SCHEMA */
+		if (HeadMatches("CREATE", "SCHEMA"))
+			COMPLETE_WITH("TABLE", "SEQUENCE");
+		else
+			COMPLETE_WITH("TABLE", "SEQUENCE", "MATERIALIZED VIEW");
 	/* Complete PARTITION BY with RANGE ( or LIST ( or ... */
 	else if (TailMatches("PARTITION", "BY"))
 		COMPLETE_WITH("RANGE (", "LIST (", "HASH (");
@@ -4263,9 +4291,7 @@ psql_completion(const char *text, int start, int end)
 /* OWNER TO  - complete with available roles */
 	else if (TailMatches("OWNER", "TO"))
 		COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_roles,
-								 "CURRENT_ROLE",
-								 "CURRENT_USER",
-								 "SESSION_USER");
+								 Keywords_for_list_of_owner_roles);
 
 /* ORDER BY */
 	else if (TailMatches("FROM", MatchAny, "ORDER"))
-- 
2.34.1


--=-=-=--