Dagfinn Ilmari Mannsåker <ilm...@ilmari.org> writes:
> Shinya Kato <shinya11.k...@gmail.com> writes:
>
>> On Fri, Aug 1, 2025 at 2:16 AM Dagfinn Ilmari Mannsåker
>> <ilm...@ilmari.org> wrote:
>>>
>>> Dagfinn Ilmari Mannsåker <ilm...@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.
I also noticed that ALTER SYSTEM RESET tab completes with all variables,
not just ones that have been set with ALTER SYSTEM. Getting this right
is a bit more complicated, since the only way to distinguish them is
pg_settings.sourcefile = '$PGDATA/postgresql.auto.conf', but Windows
uses \ for the directory separator, so we'd have to use a regex. But
there's no function for escaping a string to use as a literal match in a
regex (like Perl's quotemeta()), so we have to use LIKE instead,
manually escaping %, _ and \, and accepting any character as the
directory separator. If people think this over-complicated, we could
just check sourcefile ~ '[\\/]postgresql\.auto\.conf$', and accept false
positives if somone has used an include directive with a file with the
same name in a different directory.
Another complication is that both current_setting('data_directory') and
pg_settings.sourcefile are only available to superusers, so I added
another version for non-superusers that completes variables they've been
granted ALTER SYSTEM on, by querying pg_parameter_acl.
- ilmari
>From 623147fe3a52cf4918ee5a45aec56c5fa4f9c9e3 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 v5 1/5] 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..c3c88820dc0 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
>From be2dc71d6f8d6ad30ab96f4eec718a8e1a741c5d 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 v5 2/5] 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 c3c88820dc0..ed92e82a4b6 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
>From 5d9111d03929f8725f015b7a3496c50f1316866e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilm...@ilmari.org>
Date: Fri, 8 Aug 2025 23:06:36 +0100
Subject: [PATCH v5 3/5] 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 ed92e82a4b6..dd334d268ca 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
>From efae1fbed9591a519697bacb2730b8b1cdbba736 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilm...@ilmari.org>
Date: Fri, 8 Aug 2025 23:07:39 +0100
Subject: [PATCH v5 4/5] Improve tab completion for ALTER SYSTEM RESET
Only complete variables where sourcefile is `postgresql.conf.auto` in
the data directory.
---
src/bin/psql/tab-complete.in.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c
index dd334d268ca..60bba212f5e 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -1042,6 +1042,13 @@ static const SchemaQuery Query_for_trigger_of_table = {
" WHERE context != 'internal' "\
" AND pg_catalog.lower(name) LIKE pg_catalog.lower('%s')"
+#define Query_for_list_of_alter_system_reset_vars \
+"SELECT pg_catalog.lower(name) FROM pg_catalog.pg_settings " \
+" WHERE sourcefile LIKE pg_catalog.regexp_replace( " \
+" pg_catalog.current_setting('data_directory'), " \
+" '[_%%\\\\]', '\\\\\\&') || '_postgresql.auto.conf' " \
+" AND pg_catalog.lower(name) LIKE pg_catalog.lower('%s')"
+
#define Query_for_list_of_set_vars \
"SELECT pg_catalog.lower(name) FROM pg_catalog.pg_settings "\
" WHERE context IN ('user', 'superuser') "\
@@ -2624,8 +2631,10 @@ match_previous_words(int pattern_id,
/* ALTER SYSTEM SET, RESET, RESET ALL */
else if (Matches("ALTER", "SYSTEM"))
COMPLETE_WITH("SET", "RESET");
- else if (Matches("ALTER", "SYSTEM", "SET|RESET"))
- COMPLETE_WITH_QUERY_VERBATIM_PLUS(Query_for_list_of_alter_system_set_vars,
+ else if (Matches("ALTER", "SYSTEM", "SET"))
+ COMPLETE_WITH_QUERY_VERBATIM(Query_for_list_of_alter_system_set_vars);
+ else if (Matches("ALTER", "SYSTEM", "RESET"))
+ COMPLETE_WITH_QUERY_VERBATIM_PLUS(Query_for_list_of_alter_system_reset_vars,
"ALL");
else if (Matches("ALTER", "SYSTEM", "SET", MatchAny))
COMPLETE_WITH("TO");
--
2.50.1
>From 68883d2ea5a1880b7e8c97024a0f1cfa75d15ad9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilm...@ilmari.org>
Date: Sat, 9 Aug 2025 00:29:39 +0100
Subject: [PATCH v5 5/5] Improve tab completion for ALTER SYSTEM RESET for
non-superusers
Non-superusers can only use ALTER SYSTEM on variables they've
explicitly been GRANTed access to, and can't read the data_directory
setting or pg_settings.sourceline column, so the existing tab
completion for ALTER SYSTEM isn't very useful for them.
Instead, query pg_parameter_acl to show the variables they do have
permission to set via ALTER SYSTEM.
---
src/bin/psql/tab-complete.in.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c
index 60bba212f5e..aee939e98be 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -1049,6 +1049,13 @@ static const SchemaQuery Query_for_trigger_of_table = {
" '[_%%\\\\]', '\\\\\\&') || '_postgresql.auto.conf' " \
" AND pg_catalog.lower(name) LIKE pg_catalog.lower('%s')"
+#define Query_for_list_of_alter_system_granted_vars \
+"SELECT pg_catalog.lower(parname) FROM pg_catalog.pg_parameter_acl " \
+" WHERE EXISTS (SELECT FROM pg_catalog.aclexplode(paracl) " \
+" WHERE pg_has_role(current_role, grantee, 'usage') " \
+" AND privilege_type = 'ALTER SYSTEM') " \
+" AND pg_catalog.lower(parname) LIKE pg_catalog.lower('%s')"
+
#define Query_for_list_of_set_vars \
"SELECT pg_catalog.lower(name) FROM pg_catalog.pg_settings "\
" WHERE context IN ('user', 'superuser') "\
@@ -2632,10 +2639,16 @@ match_previous_words(int pattern_id,
else if (Matches("ALTER", "SYSTEM"))
COMPLETE_WITH("SET", "RESET");
else if (Matches("ALTER", "SYSTEM", "SET"))
- COMPLETE_WITH_QUERY_VERBATIM(Query_for_list_of_alter_system_set_vars);
+ if (is_superuser())
+ COMPLETE_WITH_QUERY_VERBATIM(Query_for_list_of_alter_system_set_vars);
+ else
+ COMPLETE_WITH_QUERY_VERBATIM(Query_for_list_of_alter_system_granted_vars);
else if (Matches("ALTER", "SYSTEM", "RESET"))
- COMPLETE_WITH_QUERY_VERBATIM_PLUS(Query_for_list_of_alter_system_reset_vars,
- "ALL");
+ if (is_superuser())
+ COMPLETE_WITH_QUERY_VERBATIM_PLUS(Query_for_list_of_alter_system_reset_vars,
+ "ALL");
+ else
+ COMPLETE_WITH_QUERY_VERBATIM(Query_for_list_of_alter_system_granted_vars);
else if (Matches("ALTER", "SYSTEM", "SET", MatchAny))
COMPLETE_WITH("TO");
/* ALTER VIEW <name> */
--
2.50.1
