Feedback-ID: i1ff147bf:Fastmail
From: =?utf-8?Q?Dagfinn_Ilmari_Manns=C3=A5ker?= <ilmari@ilmari.org>
To: Shinya Kato <shinya11.kato@gmail.com>
Cc: pgsql-hackers@postgresql.org
Subject: Re: Improve tab completion for various SET/RESET forms
References: <87bjqwwtic.fsf@wibble.ilmari.org>
	<18691637-2af2-4a2f-9f12-8201c2a88206@eisentraut.org>
	<871prqwq19.fsf@wibble.ilmari.org> <87tt4lumqz.fsf@wibble.ilmari.org>
	<87seicgu1q.fsf@wibble.ilmari.org>
	<CAOzEurSWaiE7pHhATgxMn2ok5PyQd_C+m=3TFUUmkdbTXRZLMw@mail.gmail.com>
Date: Fri, 08 Aug 2025 23:55:33 +0100
In-Reply-To: 
 <CAOzEurSWaiE7pHhATgxMn2ok5PyQd_C+m=3TFUUmkdbTXRZLMw@mail.gmail.com>
	(Shinya Kato's message of "Fri, 8 Aug 2025 12:03:30 +0900")
Message-ID: <87a549h196.fsf@wibble.ilmari.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux)
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="=-=-="
Archived-At: 
 <https://www.postgresql.org/message-id/87a549h196.fsf%40wibble.ilmari.org>
Precedence: bulk

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

Shinya Kato <shinya11.kato@gmail.com> writes:

> On Fri, Aug 1, 2025 at 2:16 AM Dagfinn Ilmari Mannsåker
> <ilmari@ilmari.org> wrote:
>>
>> Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> writes:
>>
>> > I just noticed that in addition to ALTER ROLE ... RESET being buggy, the
>> > ALTER DATABASE ... RESET query doesn't schema-qualify the unnest() call.
>> > Here's an updated patch series that fixes that too.  The first two are
>> > bug fixes to features new in 18 and should IMO be committed before
>> > that's released.  The rest can wait for 19.
>>
>> Now that Tomas has committed the two bugfixes, here's the rest of the
>> patches rebased over that.
>
> Thank you for the patches.
>
> +   else if (Matches("ALTER", "FOREIGN", "TABLE", MatchAny, "SET"))
> +       COMPLETE_WITH("SCHEMA");
>
> In addition to SET SCHEMA, I think you should add tab completion for
> SET WITHOUT OIDS.

As Kirill pointed out, support for WITH OIDS was removed in v12.  The
SET WITHOUT OIDS syntax only remains as a no-op for backwards
compatibility with existing SQL scripts, so there's no point in offering
tab completion for it.

> +#define Query_for_list_of_session_vars \
> +"SELECT pg_catalog.lower(name) FROM pg_catalog.pg_settings "\
> +" WHERE context IN ('user', 'superuser') "\
> +"   AND source = 'session' "\
> +"   AND pg_catalog.lower(name) LIKE pg_catalog.lower('%s')"
>
> I think the context IN ('user', 'superuser') condition is redundant
> here. If a parameter's source is 'session', its context must be either
> 'user' or 'superuser'. Therefore, the source = 'session' check alone
> should be sufficient.

Good point, updated in the attached v4 patches.

- ilmari


--=-=-=
Content-Type: text/x-diff
Content-Disposition: inline;
 filename=v4-0001-Add-tab-completion-for-ALTER-TABLE-.-ALTER-COLUMN.patch

From 59d8c9c4874dadc8754f0dd7ea6d6dc67f3814ff Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Mon, 9 Jun 2025 20:39:15 +0100
Subject: [PATCH v4 1/4] Add tab completion for ALTER TABLE ... ALTER COLUMN
 ... RESET

Unlike SET, it only takes parenthesised attribute options.
---
 src/bin/psql/tab-complete.in.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c
index 1f2ca946fc5..176ae1284be 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -2913,9 +2913,13 @@ match_previous_words(int pattern_id,
 					  "STATISTICS", "STORAGE",
 		/* a subset of ALTER SEQUENCE options */
 					  "INCREMENT", "MINVALUE", "MAXVALUE", "START", "NO", "CACHE", "CYCLE");
-	/* ALTER TABLE ALTER [COLUMN] <foo> SET ( */
-	else if (Matches("ALTER", "TABLE", MatchAny, "ALTER", "COLUMN", MatchAny, "SET", "(") ||
-			 Matches("ALTER", "TABLE", MatchAny, "ALTER", MatchAny, "SET", "("))
+	/* ALTER TABLE ALTER [COLUMN] <foo> RESET */
+	else if (Matches("ALTER", "TABLE", MatchAny, "ALTER", "COLUMN", MatchAny, "RESET") ||
+			 Matches("ALTER", "TABLE", MatchAny, "ALTER", MatchAny, "RESET"))
+		COMPLETE_WITH("(");
+ 	/* ALTER TABLE ALTER [COLUMN] <foo> SET|RESET ( */
+	else if (Matches("ALTER", "TABLE", MatchAny, "ALTER", "COLUMN", MatchAny, "SET|RESET", "(") ||
+			 Matches("ALTER", "TABLE", MatchAny, "ALTER", MatchAny, "SET|RESET", "("))
 		COMPLETE_WITH("n_distinct", "n_distinct_inherited");
 	/* ALTER TABLE ALTER [COLUMN] <foo> SET COMPRESSION */
 	else if (Matches("ALTER", "TABLE", MatchAny, "ALTER", "COLUMN", MatchAny, "SET", "COMPRESSION") ||
-- 
2.50.1


--=-=-=
Content-Type: text/x-diff
Content-Disposition: inline;
 filename=v4-0002-Add-tab-completion-for-ALTER-FOREIGN-TABLE-.-SET.patch

From d34caaeb8409d58b5abaac825ed1c093bb5e7cc8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Mon, 9 Jun 2025 20:41:29 +0100
Subject: [PATCH v4 2/4] Add tab completion for ALTER FOREIGN TABLE ... SET

The schema is the only thing that can be set for foreign tables.
---
 src/bin/psql/tab-complete.in.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c
index 176ae1284be..d31f5780727 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -2435,6 +2435,10 @@ match_previous_words(int pattern_id,
 		COMPLETE_WITH("ADD", "ALTER", "DISABLE TRIGGER", "DROP", "ENABLE",
 					  "INHERIT", "NO INHERIT", "OPTIONS", "OWNER TO",
 					  "RENAME", "SET", "VALIDATE CONSTRAINT");
+	else if (Matches("ALTER", "FOREIGN", "TABLE", MatchAny, "SET"))
+		COMPLETE_WITH("SCHEMA");
+	else if (Matches("ALTER", "FOREIGN", "TABLE", MatchAny, "SET", "SCHEMA"))
+		COMPLETE_WITH_QUERY(Query_for_list_of_schemas);
 
 	/* ALTER INDEX */
 	else if (Matches("ALTER", "INDEX"))
-- 
2.50.1


--=-=-=
Content-Type: text/x-diff
Content-Disposition: inline;
 filename=v4-0003-Improve-tab-completion-for-RESET.patch

From 9e3b600924c4118002fb36187fa268ba2ac37d9a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Fri, 8 Aug 2025 23:06:36 +0100
Subject: [PATCH v4 3/4] Improve tab completion for RESET

Only complete variables that have been set in the current session,
plus the keywords ALL, ROLE and SESSION (which will subsequently be
completed with AUTHORIZATION).

Because we're using Matches() insted of TailMatches(), we can also get
rid of the guards against ALTER DATABASE|USER|ROLE ... RESET.
---
 src/bin/psql/tab-complete.in.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c
index d31f5780727..c4f50880965 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -1047,6 +1047,11 @@ static const SchemaQuery Query_for_trigger_of_table = {
 " WHERE context IN ('user', 'superuser') "\
 "   AND pg_catalog.lower(name) LIKE pg_catalog.lower('%s')"
 
+#define Query_for_list_of_session_vars \
+"SELECT pg_catalog.lower(name) FROM pg_catalog.pg_settings "\
+" WHERE source = 'session' "\
+"   AND pg_catalog.lower(name) LIKE pg_catalog.lower('%s')"
+
 #define Query_for_list_of_show_vars \
 "SELECT pg_catalog.lower(name) FROM pg_catalog.pg_settings "\
 " WHERE pg_catalog.lower(name) LIKE pg_catalog.lower('%s')"
@@ -5027,9 +5032,8 @@ match_previous_words(int pattern_id,
 
 /* SET, RESET, SHOW */
 	/* Complete with a variable name */
-	else if (TailMatches("SET|RESET") &&
-			 !TailMatches("UPDATE", MatchAny, "SET") &&
-			 !TailMatches("ALTER", "DATABASE|USER|ROLE", MatchAny, "RESET"))
+	else if (TailMatches("SET") &&
+			 !TailMatches("UPDATE", MatchAny, "SET"))
 		COMPLETE_WITH_QUERY_VERBATIM_PLUS(Query_for_list_of_set_vars,
 										  "CONSTRAINTS",
 										  "TRANSACTION",
@@ -5037,6 +5041,12 @@ match_previous_words(int pattern_id,
 										  "ROLE",
 										  "TABLESPACE",
 										  "ALL");
+	/* Complete with variables set in the current session */
+	else if (Matches("RESET"))
+		COMPLETE_WITH_QUERY_VERBATIM_PLUS(Query_for_list_of_session_vars,
+										  "ALL",
+										  "ROLE",
+										  "SESSION");
 	else if (Matches("SHOW"))
 		COMPLETE_WITH_QUERY_VERBATIM_PLUS(Query_for_list_of_show_vars,
 										  "SESSION AUTHORIZATION",
-- 
2.50.1


--=-=-=--