Dagfinn Ilmari Mannsåker <ilm...@ilmari.org> writes:
> Peter Eisentraut <pe...@eisentraut.org> writes:
>
>> On 09.06.25 22:27, Dagfinn Ilmari Mannsåker wrote:
>>> I noticed that psql tab-completes every possible session-settable
>>> variable after RESET, not just the ones that have actually been set in
>>> the current session. However, as I was fixing that I noticed several
>>> other deficiencies around other forms of SET/RESET. So, here's the
>>> resulting yak stack.
>>
>> These technical changes seem fine, but I haven't looked in detail. But
>> I want to say that I don't like this proposed behavior change at all.
>
> This is not the first case of behaviour like this: there's precedence
> for it for ALTER DATABASE ... RESET and ALTER ROLE ... RESET (but that
> was buggy and is fixed by the first patch in the series): they only
> complete variables that are actually set on the relevant entity. See the
> thread at
> https://postgr.es/m/CAEP4nAzqiT6VbVC5r3nq5byLTnPzjniVGzEMpYcnAHQyNzEuaw%40mail.gmail.com
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.
- ilmari
>From 8d63ee9cbd118976ee1e5f22b9524099c21ef976 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilm...@ilmari.org>
Date: Wed, 11 Jun 2025 13:10:54 +0100
Subject: [PATCH v2 1/6] Fix unqualified unnest() call in tab completion query
Commit 9df8727c5067 added tab completion for ALTER DATABASE ... RESET,
but forgot to schema-qualifiy the unnest() call.
---
src/bin/psql/tab-complete.in.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c
index 2c0b4f28c14..85aacc316bf 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -1002,7 +1002,7 @@ static const SchemaQuery Query_for_trigger_of_table = {
#define Query_for_list_of_database_vars \
"SELECT conf FROM ("\
-" SELECT setdatabase, pg_catalog.split_part(unnest(setconfig),'=',1) conf"\
+" SELECT setdatabase, pg_catalog.split_part(pg_catalog.unnest(setconfig),'=',1) conf"\
" FROM pg_db_role_setting "\
" ) s, pg_database d "\
" WHERE s.setdatabase = d.oid "\
--
2.49.0
>From 2b0976af72bc8367599b5898b93f37cb4cc21a6a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilm...@ilmari.org>
Date: Mon, 9 Jun 2025 20:35:29 +0100
Subject: [PATCH v2 2/6] Fix tab completion for ALTER ROLE|USER ... RESET
It was showing all the right variables when hitting tab just after
RESET, but as soon as you started typing something to pick what to
complete, it would no longer work. Fix it in the same way as ALTER
DATABASE ... RESET does it.
---
src/bin/psql/tab-complete.in.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c
index 85aacc316bf..bde8c1e1c85 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -1078,9 +1078,12 @@ Keywords_for_list_of_owner_roles, "PUBLIC"
" WHERE usename LIKE '%s'"
#define Query_for_list_of_user_vars \
-" SELECT pg_catalog.split_part(pg_catalog.unnest(rolconfig),'=',1) "\
-" FROM pg_catalog.pg_roles "\
-" WHERE rolname LIKE '%s'"
+"SELECT conf FROM ("\
+" SELECT rolname, pg_catalog.split_part(pg_catalog.unnest(rolconfig),'=',1) conf"\
+" FROM pg_catalog.pg_roles"\
+" ) s"\
+" WHERE s.conf like '%s' "\
+" AND s.rolname LIKE '%s'"
#define Query_for_list_of_access_methods \
" SELECT amname "\
@@ -2495,7 +2498,10 @@ match_previous_words(int pattern_id,
/* ALTER USER,ROLE <name> RESET */
else if (Matches("ALTER", "USER|ROLE", MatchAny, "RESET"))
+ {
+ set_completion_reference(prev2_wd);
COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_user_vars, "ALL");
+ }
/* ALTER USER,ROLE <name> WITH */
else if (Matches("ALTER", "USER|ROLE", MatchAny, "WITH"))
--
2.49.0
>From 788c79bf607d7159005fe5c425d67bbbbbb7ea65 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilm...@ilmari.org>
Date: Mon, 9 Jun 2025 20:39:15 +0100
Subject: [PATCH v2 3/6] 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 bde8c1e1c85..1eaae544e12 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -2884,9 +2884,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.49.0
>From b1d3f55613545d2156db6d5b50a060f88e8c8f35 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilm...@ilmari.org>
Date: Mon, 9 Jun 2025 20:41:29 +0100
Subject: [PATCH v2 4/6] 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 1eaae544e12..6210786017e 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -2413,6 +2413,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.49.0
>From 0ae96d4df25c63173d956a366cb2049d3b02f321 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilm...@ilmari.org>
Date: Mon, 9 Jun 2025 20:45:30 +0100
Subject: [PATCH v2 5/6] Remove guard against generic completion after ALTER
DATABASE ... RESET
Commit 9df8727c5067 added explicit handling for ALTER DATABASE
... RESET earlier in the code, so no need to guard against it here.
---
src/bin/psql/tab-complete.in.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c
index 6210786017e..c3184239a17 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -4957,8 +4957,7 @@ 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", MatchAny, "RESET"))
+ !TailMatches("UPDATE", MatchAny, "SET"))
COMPLETE_WITH_QUERY_VERBATIM_PLUS(Query_for_list_of_set_vars,
"CONSTRAINTS",
"TRANSACTION",
--
2.49.0
>From 4b1459293f28b4ccf90d0ae524474613e8873f18 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilm...@ilmari.org>
Date: Mon, 9 Jun 2025 20:48:43 +0100
Subject: [PATCH v2 6/6] 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).
---
src/bin/psql/tab-complete.in.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c
index c3184239a17..4a7a86998e0 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -1039,6 +1039,12 @@ 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 context IN ('user', 'superuser') "\
+" AND 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')"
@@ -4955,8 +4961,14 @@ match_previous_words(int pattern_id,
/* naah . . . */
/* SET, RESET, SHOW */
+ /* Complete with variables set in the current session */
+ else if (TailMatches("RESET"))
+ COMPLETE_WITH_QUERY_VERBATIM_PLUS(Query_for_list_of_session_vars,
+ "ALL",
+ "ROLE",
+ "SESSION");
/* Complete with a variable name */
- else if (TailMatches("SET|RESET") &&
+ else if (TailMatches("SET") &&
!TailMatches("UPDATE", MatchAny, "SET"))
COMPLETE_WITH_QUERY_VERBATIM_PLUS(Query_for_list_of_set_vars,
"CONSTRAINTS",
--
2.49.0
