psql: Add tab completion for ALTER USER RESET
Hi,
Please find attached a patch to help tab completion show only currently set
vars
during ALTER USER RESET.
Currently tab completion provides a list of all vars which is only
partially helpful.
This patch allows tab completion to see which vars are currently set for
the role
and only show those options during tab completion.
For example:
postgres=# \drds
List of settings
Role | Database | Settings
----------+----------+-------------------------------------
postgres | | max_parallel_workers_per_gather=100+
| | max_parallel_workers=100
(1 row)
postgres=# alter user postgres reset
ALL max_parallel_workers
max_parallel_workers_per_gather
postgres=# alter user postgres reset max_parallel_workers;
ALTER ROLE
postgres=# \drds
List of settings
Role | Database | Settings
----------+----------+-------------------------------------
postgres | | max_parallel_workers_per_gather=100
(1 row)
postgres=# alter user postgres reset
ALL max_parallel_workers_per_gather
postgres=# alter user postgres reset all;
ALTER ROLE
postgres=# \drds
Did not find any settings.
Applies on master (b6612aedc53a6) / make check successful.
-
Robins Tharakan
Amazon Web Services
Attachments:
v1-0001-Add-tab-completion-for-ALTER-USER-RESET.patchapplication/octet-stream; name=v1-0001-Add-tab-completion-for-ALTER-USER-RESET.patchDownload
From a5536a66684b0540e223ce9a64da27e0ce43e744 Mon Sep 17 00:00:00 2001
From: Robins Tharakan <tharakan@gmail.com>
Date: Fri, 29 Nov 2024 19:05:38 +1030
Subject: [PATCH v1] Add tab completion for ALTER USER RESET
---
src/bin/psql/tab-complete.in.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c
index bbd08770c3..8d60408e26 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -1068,6 +1068,11 @@ Keywords_for_list_of_owner_roles, "PUBLIC"
" FROM pg_catalog.pg_user_mappings "\
" 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'"
+
#define Query_for_list_of_access_methods \
" SELECT amname "\
" FROM pg_catalog.pg_am "\
@@ -2467,6 +2472,10 @@ match_previous_words(int pattern_id,
"RENAME TO", "REPLICATION", "RESET", "SET", "SUPERUSER",
"VALID UNTIL", "WITH");
+ /* ALTER USER,ROLE <name> RESET */
+ else if (Matches("ALTER", "USER|ROLE", MatchAny, "RESET"))
+ COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_user_vars, "ALL");
+
/* ALTER USER,ROLE <name> WITH */
else if (Matches("ALTER", "USER|ROLE", MatchAny, "WITH"))
/* Similar to the above, but don't complete "WITH" again. */
--
2.39.5
Hi,
On 11/29/24 09:42, Robins Tharakan wrote:
Hi,
Please find attached a patch to help tab completion show only currently
set vars
during ALTER USER RESET.Currently tab completion provides a list of all vars which is only
partially helpful.
This patch allows tab completion to see which vars are currently set for
the role
and only show those options during tab completion.
Yeah, this looks convenient. Without the patch we'd get
...
Display all 200 possibilities? (y or n)
and then a wall of text with all the options. With the patch we get only
options that are actually set for the user, which is much better.
Two comments:
1) Does it make sense to still show "ALL" when the query returns
nothing? Not sure if we already have a way to handle this.
2) Should we do the same thing for ALTER DATABASE? That also allows
setting options.
thanks
--
Tomas Vondra
Hi Tomas,
Thanks for taking a look - apologies for the delay here.
On Tue, 10 Dec 2024 at 09:09, Tomas Vondra <tomas@vondra.me> wrote:
1) Does it make sense to still show "ALL" when the query returns
nothing? Not sure if we already have a way to handle this.
+1 - "ALL" is pointless when there is nothing to RESET, but I didn't
find an easy way to make it conditional (which was partly the
cause of delay here). For this, I tried (and not yet successful) to
create a new set of macros - something along the lines of
COMPLETE_WITH_QUERY_IFNOTEMPTY_PLUS() - where
the second part is conditional on whether the query list returns
a non-empty set.
If this is a blocker, I'll continue working on a macro that allows that
more easily (although it'd be great if you could point me to a better
way to implement that). For now, reattached v1 to this email for
convenience.
2) Should we do the same thing for ALTER DATABASE? That also allows
setting options.
+1. Attached a separate patch for ALTER DATABASE RESET in case
you think it is good to go alongside (albeit again, this still does show
ALL even if there's no vars to RESET).
-
robins
Attachments:
v1-0001-Add-tab-completion-for-ALTER-DATABASE-RESET.patchapplication/x-patch; name=v1-0001-Add-tab-completion-for-ALTER-DATABASE-RESET.patchDownload
From fd8c79cf387c1d2ce767314865e4f471a4eea81c Mon Sep 17 00:00:00 2001
From: Robins Tharakan <tharakan@gmail.com>
Date: Sat, 15 Feb 2025 21:09:17 +1030
Subject: [PATCH v1] Add tab completion for ALTER DATABASE RESET
Currently tab completion for ALTER DATABASE RESET shows a list
of all vars that could be set on a database, which is only
partially helpful. This patch allows tab completion to see which
vars are currently set for the database and only show those
options.
---
src/bin/psql/tab-complete.in.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c
index a9a81ab3c14..18fecd32807 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -1000,6 +1000,15 @@ static const SchemaQuery Query_for_trigger_of_table = {
"SELECT datname FROM pg_catalog.pg_database "\
" WHERE datname LIKE '%s'"
+#define Query_for_list_of_database_vars \
+"SELECT conf FROM ("\
+" SELECT setdatabase, pg_catalog.split_part(unnest(setconfig),'=',1) conf"\
+" FROM pg_db_role_setting "\
+" ) s, pg_database d "\
+" WHERE s.setdatabase = d.oid "\
+" AND conf LIKE '%s'"\
+" AND d.datname LIKE '%s'"
+
#define Query_for_list_of_tablespaces \
"SELECT spcname FROM pg_catalog.pg_tablespace "\
" WHERE spcname LIKE '%s'"
@@ -2320,6 +2329,13 @@ match_previous_words(int pattern_id,
"IS_TEMPLATE", "ALLOW_CONNECTIONS",
"CONNECTION LIMIT");
+ /* ALTER DATABASE <name> RESET */
+ else if (Matches("ALTER", "DATABASE", MatchAny, "RESET"))
+ {
+ set_completion_reference(prev2_wd);
+ COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_database_vars, "ALL");
+ }
+
/* ALTER DATABASE <name> SET TABLESPACE */
else if (Matches("ALTER", "DATABASE", MatchAny, "SET", "TABLESPACE"))
COMPLETE_WITH_QUERY(Query_for_list_of_tablespaces);
@@ -4906,7 +4922,9 @@ match_previous_words(int pattern_id,
/* SET, RESET, SHOW */
/* Complete with a variable name */
- else if (TailMatches("SET|RESET") && !TailMatches("UPDATE", MatchAny, "SET"))
+ else if (TailMatches("SET|RESET") &&
+ !TailMatches("UPDATE", MatchAny, "SET") &&
+ !TailMatches("ALTER", "DATABASE", MatchAny, "RESET"))
COMPLETE_WITH_QUERY_VERBATIM_PLUS(Query_for_list_of_set_vars,
"CONSTRAINTS",
"TRANSACTION",
--
2.43.0
v1-0001-Add-tab-completion-for-ALTER-USER-RESET.patchapplication/x-patch; name=v1-0001-Add-tab-completion-for-ALTER-USER-RESET.patchDownload
From a5536a66684b0540e223ce9a64da27e0ce43e744 Mon Sep 17 00:00:00 2001
From: Robins Tharakan <tharakan@gmail.com>
Date: Fri, 29 Nov 2024 19:05:38 +1030
Subject: [PATCH v1] Add tab completion for ALTER USER RESET
---
src/bin/psql/tab-complete.in.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c
index bbd08770c3..8d60408e26 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -1068,6 +1068,11 @@ Keywords_for_list_of_owner_roles, "PUBLIC"
" FROM pg_catalog.pg_user_mappings "\
" 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'"
+
#define Query_for_list_of_access_methods \
" SELECT amname "\
" FROM pg_catalog.pg_am "\
@@ -2467,6 +2472,10 @@ match_previous_words(int pattern_id,
"RENAME TO", "REPLICATION", "RESET", "SET", "SUPERUSER",
"VALID UNTIL", "WITH");
+ /* ALTER USER,ROLE <name> RESET */
+ else if (Matches("ALTER", "USER|ROLE", MatchAny, "RESET"))
+ COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_user_vars, "ALL");
+
/* ALTER USER,ROLE <name> WITH */
else if (Matches("ALTER", "USER|ROLE", MatchAny, "WITH"))
/* Similar to the above, but don't complete "WITH" again. */
--
2.39.5
On 2/15/25 12:14, Robins Tharakan wrote:
Hi Tomas,
Thanks for taking a look - apologies for the delay here.
On Tue, 10 Dec 2024 at 09:09, Tomas Vondra <tomas@vondra.me
<mailto:tomas@vondra.me>> wrote:1) Does it make sense to still show "ALL" when the query returns
nothing? Not sure if we already have a way to handle this.+1 - "ALL" is pointless when there is nothing to RESET, but I didn't
find an easy way to make it conditional (which was partly the
cause of delay here). For this, I tried (and not yet successful) to
create a new set of macros - something along the lines of
COMPLETE_WITH_QUERY_IFNOTEMPTY_PLUS() - where
the second part is conditional on whether the query list returns
a non-empty set.If this is a blocker, I'll continue working on a macro that allows that
more easily (although it'd be great if you could point me to a better
way to implement that). For now, reattached v1 to this email for
convenience.
Thanks. I don't think it's a blocker - in fact, after thinking about it
a bit more, I believe showing the "ALL" is consistent with what we do in
other places when the query returns nothing. Consider for example CLOSE:
/* CLOSE */
else if (Matches("CLOSE"))
COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_cursors,
"ALL");
Other places do similar stuff. It's just a noop. So I don't think you
need to waste time inventing something for this one place.
2) Should we do the same thing for ALTER DATABASE? That also allows
setting options.+1. Attached a separate patch for ALTER DATABASE RESET in case
you think it is good to go alongside (albeit again, this still does show
ALL even if there's no vars to RESET).
Thanks. These patches look fine to me. I'll get them committed.
regards
--
Tomas Vondra