[PATCH] psql: tab completion for ALTER ROLE ... IN DATABASE ...
Hi
I found myself needing to work with ALTER ROLE ... IN DATABASE ... recently
and was annoyed by the lack of tab completion for this, so patch attached.
Regards
Ian Barwick
Attachments:
v1-0001-psql-add-tab-completion-for-ALTER-ROLE-IN-DATABASE.patchapplication/x-patch; name=v1-0001-psql-add-tab-completion-for-ALTER-ROLE-IN-DATABASE.patchDownload
From de1cb4cb546b9cfac280c44aea2cf112a85bc7e9 Mon Sep 17 00:00:00 2001
From: Ian Barwick <barwick@gmail.com>
Date: Fri, 21 Nov 2025 11:47:26 +0900
Subject: [PATCH v1] psql: add tab completion for ALTER ROLE ... IN DATABASE
...
This patch adds tab completion for the "IN DATABASE" clause of ALTER
ROLE.
---
src/bin/psql/tab-complete.in.c | 26 ++++++++++++++++++++++++--
1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c
index 51806597037..1d03d8eb49e 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -2536,12 +2536,34 @@ match_previous_words(int pattern_id,
else if (Matches("ALTER", "USER|ROLE", MatchAny) &&
!TailMatches("USER", "MAPPING"))
COMPLETE_WITH("BYPASSRLS", "CONNECTION LIMIT", "CREATEDB", "CREATEROLE",
- "ENCRYPTED PASSWORD", "INHERIT", "LOGIN", "NOBYPASSRLS",
- "NOCREATEDB", "NOCREATEROLE", "NOINHERIT",
+ "ENCRYPTED PASSWORD", "IN DATABASE", "INHERIT", "LOGIN",
+ "NOBYPASSRLS", "NOCREATEDB", "NOCREATEROLE", "NOINHERIT",
"NOLOGIN", "NOREPLICATION", "NOSUPERUSER", "PASSWORD",
"RENAME TO", "REPLICATION", "RESET", "SET", "SUPERUSER",
"VALID UNTIL", "WITH");
+ /* ALTER USER,ROLE <name> IN */
+ else if (Matches("ALTER", "USER|ROLE", MatchAny, "IN"))
+ COMPLETE_WITH("DATABASE");
+
+ /* ALTER USER,ROLE <name> IN DATABASE */
+ else if (HeadMatches("ALTER", "USER|ROLE", MatchAny, "IN"))
+ {
+ if (TailMatches("DATABASE"))
+ COMPLETE_WITH_QUERY(Query_for_list_of_databases);
+
+ else if (TailMatches("DATABASE", MatchAny))
+ COMPLETE_WITH("SET", "RESET");
+
+ else if (TailMatches("DATABASE", MatchAny, "SET"))
+ COMPLETE_WITH_QUERY(Query_for_list_of_set_vars);
+
+ else if (TailMatches("DATABASE", MatchAny, "RESET"))
+ {
+ set_completion_reference(prev5_wd);
+ COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_user_vars, "ALL");
+ }
+ }
/* ALTER USER,ROLE <name> RESET */
else if (Matches("ALTER", "USER|ROLE", MatchAny, "RESET"))
{
base-commit: 266543a62055541ddefe4e66797e0e1e4aa8705a
--
2.43.0
On Fri, Nov 21, 2025 at 1:25 PM Ian Lawrence Barwick <barwick@gmail.com>
wrote:
Hi
I found myself needing to work with ALTER ROLE ... IN DATABASE ... recently
and was annoyed by the lack of tab completion for this, so patch attached.Regards
Ian Barwick
HI, I've reviewed the patch and did simple tests — it works correctly.
Ian Lawrence Barwick <barwick@gmail.com> writes:
Hi
I found myself needing to work with ALTER ROLE ... IN DATABASE ... recently
and was annoyed by the lack of tab completion for this, so patch attached.
A noble goal, but unfortunately th RESET form can't work properly due to
limitations of the tab completion system.
+ /* ALTER USER,ROLE <name> IN DATABASE */ + else if (HeadMatches("ALTER", "USER|ROLE", MatchAny, "IN")) + {
[...]
+ else if (TailMatches("DATABASE", MatchAny, "RESET")) + { + set_completion_reference(prev5_wd); + COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_user_vars, "ALL");
This queries pg_roles.rolconfig, which only contains variables set for
the user in all databases, not in the specified database. Instead,
you'd need to query pg_db_role_setting WHERE setdatabase = (SELECT oid
FROM pg_database WHERE datname = '%s') AND setrole = '%s'::regrole, but
unfortunately the tab completion system doesn't let you more than one
previous word in the query. I guess you could query WHERE setdatabase
<> 0, to get variables set for the user across all databases, not just
the specified one.
Also, alter ALTER ROLE ALL RESET needs separate handling, filtering
where setrole = 0, which is actually possible in the current system.
- ilmari
Hi Ian,
+1 for the patch,LGTM
But after applying the patch i can be able to apply all the results
except the reset options user variables
postgres=#
postgres=# alter role bob
BYPASSRLS CREATEROLE INHERIT NOCREATEDB
NOLOGIN PASSWORD RESET
VALID UNTIL
CONNECTION LIMIT ENCRYPTED PASSWORD LOGIN
NOCREATEROLE NOREPLICATION RENAME TO SET
WITH
CREATEDB IN DATABASE NOBYPASSRLS NOINHERIT
NOSUPERUSER REPLICATION SUPERUSER
postgres=# alter role bob in DATABASE
postgres template0 template1
postgres=# alter role bob in DATABASE postgres
RESET SET
postgres=# alter role bob in DATABASE postgres reset ALL
postgres=# alter role bob in DATABASE postgres reset ALL
also i cross verified that my system doesn't have variables so it
returns 0 rows?but:
postgres=# SELECT name FROM pg_settings LIMIT 5;
name
----------------------------
allow_alter_system
allow_in_place_tablespaces
allow_system_table_mods
application_name
archive_cleanup_command
(5 rows)
Can you check this ?
-regards
Vasuki M
Show quoted text
On Fri, Nov 21, 2025 at 8:44 AM Ian Lawrence Barwick <barwick@gmail.com> wrote:
Hi
I found myself needing to work with ALTER ROLE ... IN DATABASE ... recently
and was annoyed by the lack of tab completion for this, so patch attached.Regards
Ian Barwick
Hi BharatDB,
On Tue, Nov 25, 2025 at 5:15 PM BharatDB <bharatdbpg@gmail.com> wrote:
Hi Ian,
+1 for the patch,LGTM
But after applying the patch i can be able to apply all the results
except the reset options user variablespostgres=# alter role bob in DATABASE postgres reset ALL
also i cross verified that my system doesn't have variables so it
returns 0 rows?but:postgres=# SELECT name FROM pg_settings LIMIT 5;
name
----------------------------
allow_alter_system
allow_in_place_tablespaces
allow_system_table_mods
application_name
archive_cleanup_command
(5 rows)
The tab-completion here queries the user-specific config (not the global
pg_settings). I believe the underlying code logic will help explain this
behavior:
+ else if (TailMatches("DATABASE", MatchAny, "RESET")) + { + set_completion_reference(prev5_wd); + COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_user_vars, "ALL"); + }
#define Query_for_list_of_user_vars \
Show quoted text
"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'"
Hello all,
Based on the discussion, I have updated the patch to handle the RESET form
correctly without modifying psql’s tab-completion APIs.
Both the current database connection and the parsed input tokens are
already available via pset.db and the word tokens.The new patch extracts:
-
the role name from the parsed tokens (prev5_wd), and
-
the database name from the parsed tokens (prev2_wd),
and uses these to query pg_db_role_setting for variables that are actually
set for the specific (role, database) pair.
Literal quoting is now done with PQescapeLiteral(pset.db, …), per libpq
conventions, so no new helper functions were needed.
SET
After
ALTER ROLE <role> IN DATABASE <dbname> SET <TAB>
psql completes from Query_for_list_of_set_vars (same behavior as plain
ALTER ROLE … SET).
RESET
After
ALTER ROLE <role> IN DATABASE <dbname> RESET <TAB>
psql now completes with the GUC names recorded in pg_db_role_setting for
that specific (role,database), plus ALL.
When no settings exist, only ALL is suggested.
This mirrors the existing behavior of ALTER DATABASE … RESET.
I have attached the patch.
Regards,
Vasuki
On Thu, Nov 27, 2025 at 2:27 PM Ian Lawrence Barwick <barwick@gmail.com>
wrote:
Show quoted text
Hi
I found myself needing to work with ALTER ROLE ... IN DATABASE ... recently
and was annoyed by the lack of tab completion for this, so patch attached.Regards
Ian Barwick
Attachments:
0001-psql-Add-tab-completion-support-for.patchtext/x-patch; charset=UTF-8; name=0001-psql-Add-tab-completion-support-for.patchDownload
From 3897e315544445e866baeb7806df77f08cd211c9 Mon Sep 17 00:00:00 2001
From: BharatDBPG <bharatdbpg@gmail.com>
Date: Thu, 27 Nov 2025 14:38:48 +0530
Subject: [PATCH] psql: Add tab-completion support for ALTER ROLE ... IN
DATABASE ... SET/RESET forms
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This patch extends psql's tab-completion logic to recognize the
"ALTER ROLE <role> IN DATABASE <dbname>" command structure, and
provides appropriate completions for the SET and RESET subcommands.
Specifically:
• After "ALTER ROLE <role> IN DATABASE <dbname> SET", psql now
completes with the list of configuration variables that may be set
(Query_for_list_of_set_vars), matching the behavior of the plain
ALTER ROLE ... SET form.
• After "ALTER ROLE <role> IN DATABASE <dbname> RESET", psql now
suggests configuration variables that are *actually set* for that
specific (role,database) pair, as recorded in pg_db_role_setting,
plus the keyword ALL. This mirrors the behavior of ALTER DATABASE
... RESET, where we complete only the variables currently set for
the object being modified.
The role name and database name are extracted from the already-parsed
input tokens, and SQL literal quoting is performed via PQescapeLiteral()
using the implicit PGconn (pset.db) available to the tab-completion code.
This avoids any need to alter tab-completion APIs and keeps the patch
self-contained.
Due to the structure of tab-completion, this patch intentionally does
not attempt to complete arbitrary GUC names for RESET, but rather only
those that exist in pg_db_role_setting for the given role and
database. When none are present, psql falls back to suggesting ALL,
matching existing RESET behavior elsewhere.
Vasuki M
---
src/bin/psql/tab-complete.in.c | 66 ++++++++++++++++++++++++++++++++--
1 file changed, 64 insertions(+), 2 deletions(-)
diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c
index 51806597037..ca0e64b7bcf 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -2536,12 +2536,74 @@ match_previous_words(int pattern_id,
else if (Matches("ALTER", "USER|ROLE", MatchAny) &&
!TailMatches("USER", "MAPPING"))
COMPLETE_WITH("BYPASSRLS", "CONNECTION LIMIT", "CREATEDB", "CREATEROLE",
- "ENCRYPTED PASSWORD", "INHERIT", "LOGIN", "NOBYPASSRLS",
- "NOCREATEDB", "NOCREATEROLE", "NOINHERIT",
+ "ENCRYPTED PASSWORD", "IN DATABASE", "INHERIT", "LOGIN",
+ "NOBYPASSRLS", "NOCREATEDB", "NOCREATEROLE", "NOINHERIT",
"NOLOGIN", "NOREPLICATION", "NOSUPERUSER", "PASSWORD",
"RENAME TO", "REPLICATION", "RESET", "SET", "SUPERUSER",
"VALID UNTIL", "WITH");
+ /* ALTER USER,ROLE <name> IN */
+ else if (Matches("ALTER", "USER|ROLE", MatchAny, "IN"))
+ COMPLETE_WITH("DATABASE");
+
+ /* ALTER USER/ROLE <name> IN DATABASE */
+ else if (Matches("ALTER", "USER|ROLE", MatchAny, "IN", "DATABASE", MatchAny))
+ {
+ /* ALTER ROLE bob IN DATABASE <TAB> → list databases */
+ if (TailMatches("DATABASE"))
+ COMPLETE_WITH_QUERY(Query_for_list_of_databases);
+
+ /* ALTER ROLE bob IN DATABASE mydb <TAB> → SET, RESET */
+ else if (TailMatches("DATABASE", MatchAny))
+ COMPLETE_WITH("SET", "RESET");
+
+ /* ALTER ROLE bob IN DATABASE mydb SET <TAB> */
+ else if (TailMatches("DATABASE", MatchAny, "SET"))
+ COMPLETE_WITH_QUERY(Query_for_list_of_set_vars);
+
+ /* ALTER ROLE bob IN DATABASE mydb RESET <TAB> */
+ else if (TailMatches("DATABASE", MatchAny, "RESET"))
+ {
+ /*
+ * Extract tokens:
+ * prev5 = role name
+ * prev2 = database name
+ */
+ char *role = prev5_wd;
+ char *dbname = prev2_wd;
+ char *q_role;
+ char *q_dbname;
+ char *query;
+
+ /* Safe SQL literal quoting using libpq */
+ q_role = PQescapeLiteral(pset.db, role, strlen(role));
+ q_dbname = PQescapeLiteral(pset.db, dbname, strlen(dbname));
+
+ if (!q_role || !q_dbname)
+ {
+ /* If quoting fails, just fall back to ALL */
+ COMPLETE_WITH("ALL");
+ }
+ else
+ {
+
+ query = psprintf(
+ "SELECT unnest(setconfig) "
+ " FROM pg_db_role_setting "
+ " WHERE setdatabase = "
+ " (SELECT oid FROM pg_database WHERE datname = %s) "
+ " AND setrole = %s::regrole",
+ q_dbname, q_role);
+
+ COMPLETE_WITH_QUERY_PLUS(query, "ALL");
+
+ pfree(q_role);
+ pfree(q_dbname);
+ pfree(query);
+ }
+ }
+ }
+
/* ALTER USER,ROLE <name> RESET */
else if (Matches("ALTER", "USER|ROLE", MatchAny, "RESET"))
{
--
2.43.0
Kindly review the attached patch ,
As i saw the proposed patch by Ian failed with CI
https://commitfest.postgresql.org/patch/6244/
Guide me with the patch
Regards,
Vasuki M
On Thu, Nov 27, 2025 at 2:50 PM VASUKI M <vasukianand0119@gmail.com> wrote:
Show quoted text
Hello all,
Based on the discussion, I have updated the patch to handle the RESET form
correctly without modifying psql’s tab-completion APIs.Both the current database connection and the parsed input tokens are
already available via pset.db and the word tokens.The new patch extracts:-
the role name from the parsed tokens (prev5_wd), and
-the database name from the parsed tokens (prev2_wd),
and uses these to query pg_db_role_setting for variables that are actually
set for the specific (role, database) pair.Literal quoting is now done with PQescapeLiteral(pset.db, …), per libpq
conventions, so no new helper functions were needed.SET
AfterALTER ROLE <role> IN DATABASE <dbname> SET <TAB>
psql completes from Query_for_list_of_set_vars (same behavior as plain
ALTER ROLE … SET).RESET
AfterALTER ROLE <role> IN DATABASE <dbname> RESET <TAB>
psql now completes with the GUC names recorded in pg_db_role_setting for
that specific (role,database), plus ALL.
When no settings exist, only ALL is suggested.
This mirrors the existing behavior of ALTER DATABASE … RESET.I have attached the patch.
Regards,
Vasuki
On Thu, Nov 27, 2025 at 2:27 PM Ian Lawrence Barwick <barwick@gmail.com>
wrote:Hi
I found myself needing to work with ALTER ROLE ... IN DATABASE ...
recently
and was annoyed by the lack of tab completion for this, so patch attached.Regards
Ian Barwick
Attachments:
0001-psql-Add-tab-completion-support-for.patchtext/x-patch; charset=UTF-8; name=0001-psql-Add-tab-completion-support-for.patchDownload
From 3897e315544445e866baeb7806df77f08cd211c9 Mon Sep 17 00:00:00 2001
From: BharatDBPG <bharatdbpg@gmail.com>
Date: Thu, 27 Nov 2025 14:38:48 +0530
Subject: [PATCH] psql: Add tab-completion support for ALTER ROLE ... IN
DATABASE ... SET/RESET forms
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This patch extends psql's tab-completion logic to recognize the
"ALTER ROLE <role> IN DATABASE <dbname>" command structure, and
provides appropriate completions for the SET and RESET subcommands.
Specifically:
• After "ALTER ROLE <role> IN DATABASE <dbname> SET", psql now
completes with the list of configuration variables that may be set
(Query_for_list_of_set_vars), matching the behavior of the plain
ALTER ROLE ... SET form.
• After "ALTER ROLE <role> IN DATABASE <dbname> RESET", psql now
suggests configuration variables that are *actually set* for that
specific (role,database) pair, as recorded in pg_db_role_setting,
plus the keyword ALL. This mirrors the behavior of ALTER DATABASE
... RESET, where we complete only the variables currently set for
the object being modified.
The role name and database name are extracted from the already-parsed
input tokens, and SQL literal quoting is performed via PQescapeLiteral()
using the implicit PGconn (pset.db) available to the tab-completion code.
This avoids any need to alter tab-completion APIs and keeps the patch
self-contained.
Due to the structure of tab-completion, this patch intentionally does
not attempt to complete arbitrary GUC names for RESET, but rather only
those that exist in pg_db_role_setting for the given role and
database. When none are present, psql falls back to suggesting ALL,
matching existing RESET behavior elsewhere.
Vasuki M
---
src/bin/psql/tab-complete.in.c | 66 ++++++++++++++++++++++++++++++++--
1 file changed, 64 insertions(+), 2 deletions(-)
diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c
index 51806597037..ca0e64b7bcf 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -2536,12 +2536,74 @@ match_previous_words(int pattern_id,
else if (Matches("ALTER", "USER|ROLE", MatchAny) &&
!TailMatches("USER", "MAPPING"))
COMPLETE_WITH("BYPASSRLS", "CONNECTION LIMIT", "CREATEDB", "CREATEROLE",
- "ENCRYPTED PASSWORD", "INHERIT", "LOGIN", "NOBYPASSRLS",
- "NOCREATEDB", "NOCREATEROLE", "NOINHERIT",
+ "ENCRYPTED PASSWORD", "IN DATABASE", "INHERIT", "LOGIN",
+ "NOBYPASSRLS", "NOCREATEDB", "NOCREATEROLE", "NOINHERIT",
"NOLOGIN", "NOREPLICATION", "NOSUPERUSER", "PASSWORD",
"RENAME TO", "REPLICATION", "RESET", "SET", "SUPERUSER",
"VALID UNTIL", "WITH");
+ /* ALTER USER,ROLE <name> IN */
+ else if (Matches("ALTER", "USER|ROLE", MatchAny, "IN"))
+ COMPLETE_WITH("DATABASE");
+
+ /* ALTER USER/ROLE <name> IN DATABASE */
+ else if (Matches("ALTER", "USER|ROLE", MatchAny, "IN", "DATABASE", MatchAny))
+ {
+ /* ALTER ROLE bob IN DATABASE <TAB> → list databases */
+ if (TailMatches("DATABASE"))
+ COMPLETE_WITH_QUERY(Query_for_list_of_databases);
+
+ /* ALTER ROLE bob IN DATABASE mydb <TAB> → SET, RESET */
+ else if (TailMatches("DATABASE", MatchAny))
+ COMPLETE_WITH("SET", "RESET");
+
+ /* ALTER ROLE bob IN DATABASE mydb SET <TAB> */
+ else if (TailMatches("DATABASE", MatchAny, "SET"))
+ COMPLETE_WITH_QUERY(Query_for_list_of_set_vars);
+
+ /* ALTER ROLE bob IN DATABASE mydb RESET <TAB> */
+ else if (TailMatches("DATABASE", MatchAny, "RESET"))
+ {
+ /*
+ * Extract tokens:
+ * prev5 = role name
+ * prev2 = database name
+ */
+ char *role = prev5_wd;
+ char *dbname = prev2_wd;
+ char *q_role;
+ char *q_dbname;
+ char *query;
+
+ /* Safe SQL literal quoting using libpq */
+ q_role = PQescapeLiteral(pset.db, role, strlen(role));
+ q_dbname = PQescapeLiteral(pset.db, dbname, strlen(dbname));
+
+ if (!q_role || !q_dbname)
+ {
+ /* If quoting fails, just fall back to ALL */
+ COMPLETE_WITH("ALL");
+ }
+ else
+ {
+
+ query = psprintf(
+ "SELECT unnest(setconfig) "
+ " FROM pg_db_role_setting "
+ " WHERE setdatabase = "
+ " (SELECT oid FROM pg_database WHERE datname = %s) "
+ " AND setrole = %s::regrole",
+ q_dbname, q_role);
+
+ COMPLETE_WITH_QUERY_PLUS(query, "ALL");
+
+ pfree(q_role);
+ pfree(q_dbname);
+ pfree(query);
+ }
+ }
+ }
+
/* ALTER USER,ROLE <name> RESET */
else if (Matches("ALTER", "USER|ROLE", MatchAny, "RESET"))
{
--
2.43.0
Hi All,
Thanks to Ian and Vasuki for working on tab-completion support for ALTER
ROLE ... IN DATABASE.
This is a really good improvement.
Regarding Ian’s patch:
1. The patch applies cleanly and works as expected.
2. Tab-completion for IN DATABASE behaves consistently with existing ALTER
ROLE forms.
Suggestion: consider adding regression tests in "src/bin/psql/t/" to cover
these new cases.
Regarding Vasuki’s patch:
1. Really nice idea to extend completion for RESET by querying
pg_db_role_setting.
2. Use of PQescapeLiteral() (safe quoting) and consistent fallback to ALL
is really nice, and if the user falls back to 'ALL' maybe it is a good idea
to log it or let the user know of it.
Suggestion: consider adding regression tests in "src/bin/psql/t/" to cover
these new cases for SET/RESET.
Overall both patches look great.
Regards,
Surya
Hi surya,
Thanks a lot for reviewing the patches and for the encouraging feedback
Sorry for the late reply — I missed the mail since I wasn’t CC’d and
noticed it only now.
Good point about the TAP tests. I'll be adding tab-completion tests in
src/bin/psql/t/010_tab_completion.pl for both Ian’s patch and the SET/RESET
cases from my patch shortly.
Thanks again!
Regards,
Vasuki M
C-DAC,Chennai
Hi
I noticed that in the code, the variables `q_role` and `q_dbname` are processed with the `PQescapeLiteral` function,
so `PQfreemem` – instead of `pfree` – should be used here to free the memory.
--
Regards,
Man Zeng
www.openhalo.org
Hi zeng,
Thanks for pointing this out. You’re absolutely right — PQescapeLiteral()
allocates memory using libpq’s allocator, so the returned buffers must be
released with PQfreemem() rather than pfree(). Using pfree() here would be
incorrect, since it expects memory allocated via PostgreSQL’s memory
context APIs (palloc/psprintf).
I’ll update the patch to replace pfree() with PQfreemem() for the buffers
returned by PQescapeLiteral(),while continuing to use pfree() for memory
allocated via psprintf().
Thanks again for catching this.
Best regards,
Vasuki M
C-DAC,Chennai
On Mon, 22 Dec 2025, 8:18 pm zengman, <zengman@halodbtech.com> wrote:
Show quoted text
Hi
I noticed that in the code, the variables `q_role` and `q_dbname` are
processed with the `PQescapeLiteral` function,
so `PQfreemem` – instead of `pfree` – should be used here to free the
memory.--
Regards,
Man Zeng
www.openhalo.org
Hi all,
I tried adding TAP coverage for the new ALTER ROLE … IN
DATABASE tab-completion paths in src/bin/psql/t/010_tab_completion.pl.
The tests themselves work as expected, but I ran into a limitation of the
existing interactive completion harness. The new RESET completion
intentionally ends on incomplete SQL and leaves psql in
continuation/readline mode (postgres-#). As a result, the interactive psql
process does not terminate cleanly at the end of the test, causing
IPC::Run to time out and the test to abort, even though all completion
checks pass.
Earlier completion tests never exercised this state, so the harness
implicitly assumes that psql can always be exited cleanly after <TAB> using
\q / clear query(); / clear line(); . This change exposes that assumption
rather than introducing a regression.
Given this limitation, and to avoid relying on timeouts or fragile cleanup
logic, I’m omitting TAP tests for this change for now. If there’s interest
in extending or refactoring the completion test harness to better handle
continuation-mode cases, I’d be happy to look into that separately.
Attaching some lines from the logfile for the reference,
[11:19:45.497](0.000s) ok 79 - complete a psql variable name
[11:19:45.497](0.000s) ok 80 - complete a psql variable value
[11:19:45.498](0.000s) ok 81 - \r works
[11:19:45.498](0.000s) ok 82 - complete an interpolated psql variable name
[11:19:45.498](0.000s) ok 83 - \r works
[11:19:45.498](0.000s) ok 84 - complete a psql variable test
[11:19:45.498](0.000s) ok 85 - \r works
[11:19:45.498](0.000s) ok 86 - check completion failure path
[11:19:45.499](0.000s) ok 87 - \r works
[11:19:45.499](0.000s) ok 88 - COPY FROM with DEFAULT completion
[11:19:45.499](0.000s) ok 89 - control-U works
IPC::Run: timeout on timer #1 at /usr/share/perl5/IPC/Run.pm line 3007.
# Postmaster PID for node "main" is 16601
### Stopping node "main" using mode immediate
# Running: pg_ctl --pgdata
/home/cdac/postgres/src/bin/psql/tmp_check/t_010_tab_completion_main_data/pgdata
--mode immediate stop
waiting for server to shut down.... done
server stopped
# No postmaster PID for node "main"
[11:22:46.573](181.074s) # Tests were run but no plan was declared and
done_testing() was not seen.
[11:22:46.574](0.000s) # Looks like your test exited with 29 just after 89.
Regards,
Vasuki M
C-DAC,Chennai.
On Mon, Dec 22, 2025 at 9:10 PM VASUKI M <vasukianand0119@gmail.com> wrote:
Show quoted text
Hi zeng,
Thanks for pointing this out. You’re absolutely right — PQescapeLiteral()
allocates memory using libpq’s allocator, so the returned buffers must be
released with PQfreemem() rather than pfree(). Using pfree() here would be
incorrect, since it expects memory allocated via PostgreSQL’s memory
context APIs (palloc/psprintf).I’ll update the patch to replace pfree() with PQfreemem() for the buffers
returned by PQescapeLiteral(),while continuing to use pfree() for memory
allocated via psprintf().Thanks again for catching this.
Best regards,
Vasuki M
C-DAC,ChennaiOn Mon, 22 Dec 2025, 8:18 pm zengman, <zengman@halodbtech.com> wrote:
Hi
I noticed that in the code, the variables `q_role` and `q_dbname` are
processed with the `PQescapeLiteral` function,
so `PQfreemem` – instead of `pfree` – should be used here to free the
memory.--
Regards,
Man Zeng
www.openhalo.org
Attachments:
v2-0001-psql-alter-role-in-database-tab-completion.patchtext/x-patch; charset=UTF-8; name=v2-0001-psql-alter-role-in-database-tab-completion.patchDownload
From 11fa95492f79c3104534cba5b616b2aeafbda6be Mon Sep 17 00:00:00 2001
From: BharatDBPG <bharatdbpg@gmail.com>
Date: Thu, 27 Nov 2025 14:38:48 +0530
Subject: [PATCH] psql: Add tab-completion support for ALTER ROLE ... IN
DATABASE ... SET/RESET forms
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This patch extends psql's tab-completion logic to recognize the
"ALTER ROLE <role> IN DATABASE <dbname>" command structure, and
provides appropriate completions for the SET and RESET subcommands.
Specifically:
• After "ALTER ROLE <role> IN DATABASE <dbname> SET", psql now
completes with the list of configuration variables that may be set
(Query_for_list_of_set_vars), matching the behavior of the plain
ALTER ROLE ... SET form.
• After "ALTER ROLE <role> IN DATABASE <dbname> RESET", psql now
suggests configuration variables that are *actually set* for that
specific (role,database) pair, as recorded in pg_db_role_setting,
plus the keyword ALL. This mirrors the behavior of ALTER DATABASE
... RESET, where we complete only the variables currently set for
the object being modified.
The role name and database name are extracted from the already-parsed
input tokens, and SQL literal quoting is performed via PQescapeLiteral()
using the implicit PGconn (pset.db) available to the tab-completion code.
This avoids any need to alter tab-completion APIs and keeps the patch
self-contained.
Due to the structure of tab-completion, this patch intentionally does
not attempt to complete arbitrary GUC names for RESET, but rather only
those that exist in pg_db_role_setting for the given role and
database. When none are present, psql falls back to suggesting ALL,
matching existing RESET behavior elsewhere.
Vasuki M
---
src/bin/psql/tab-complete.in.c | 66 ++++++++++++++++++++++++++++++++--
1 file changed, 64 insertions(+), 2 deletions(-)
diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c
index 75a101c6ab5..c4cb79c33f2 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -2536,12 +2536,74 @@ match_previous_words(int pattern_id,
else if (Matches("ALTER", "USER|ROLE", MatchAny) &&
!TailMatches("USER", "MAPPING"))
COMPLETE_WITH("BYPASSRLS", "CONNECTION LIMIT", "CREATEDB", "CREATEROLE",
- "ENCRYPTED PASSWORD", "INHERIT", "LOGIN", "NOBYPASSRLS",
- "NOCREATEDB", "NOCREATEROLE", "NOINHERIT",
+ "ENCRYPTED PASSWORD", "IN DATABASE", "INHERIT", "LOGIN",
+ "NOBYPASSRLS", "NOCREATEDB", "NOCREATEROLE", "NOINHERIT",
"NOLOGIN", "NOREPLICATION", "NOSUPERUSER", "PASSWORD",
"RENAME TO", "REPLICATION", "RESET", "SET", "SUPERUSER",
"VALID UNTIL", "WITH");
+ /* ALTER USER,ROLE <name> IN */
+ else if (Matches("ALTER", "USER|ROLE", MatchAny, "IN"))
+ COMPLETE_WITH("DATABASE");
+ /* ALTER USER/ROLE <name> IN DATABASE */
+ else if (Matches("ALTER", "USER|ROLE", MatchAny, "IN", "DATABASE"))
+ {
+ /* ALTER ROLE bob IN DATABASE <TAB> → list databases */
+ COMPLETE_WITH_QUERY(Query_for_list_of_databases);
+ }
+ /* ALTER USER/ROLE <name> IN DATABASE <dbname> */
+ else if (Matches("ALTER", "USER|ROLE", MatchAny, "IN", "DATABASE", MatchAny))
+ {
+ /* ALTER ROLE bob IN DATABASE mydb <TAB> → SET, RESET */
+ COMPLETE_WITH("SET", "RESET");
+ }
+ /* ALTER USER/ROLE <name> IN DATABASE <dbname> SET */
+ else if (Matches("ALTER", "USER|ROLE", MatchAny, "IN", "DATABASE", MatchAny, "SET"))
+ {
+ /* ALTER ROLE bob IN DATABASE mydb SET <TAB> */
+ COMPLETE_WITH_QUERY(Query_for_list_of_set_vars);
+ }
+ /* ALTER USER/ROLE <name> IN DATABASE <dbname> RESET */
+ else if (Matches("ALTER", "USER|ROLE", MatchAny, "IN", "DATABASE", MatchAny, "RESET"))
+ {
+ /*
+ * Extract tokens:
+ * prev5 = role name
+ * prev2 = database name
+ */
+ char *role = prev5_wd;
+ char *dbname = prev2_wd;
+ char *q_role;
+ char *q_dbname;
+ char *query;
+ /* Safe SQL literal quoting using libpq */
+ q_role = PQescapeLiteral(pset.db, role, strlen(role));
+ q_dbname = PQescapeLiteral(pset.db, dbname, strlen(dbname));
+ if (!q_role || !q_dbname)
+ {
+ /* If quoting fails, just fall back to ALL */
+ if (q_role)
+ PQfreemem(q_role);
+ if (q_dbname)
+ PQfreemem(q_dbname);
+ COMPLETE_WITH("ALL");
+ }
+ else
+ {
+ query = psprintf(
+ "SELECT unnest(setconfig) "
+ " FROM pg_db_role_setting "
+ " WHERE setdatabase = "
+ " (SELECT oid FROM pg_database WHERE datname = %s) "
+ " AND setrole = %s::regrole",
+ q_dbname, q_role);
+ COMPLETE_WITH_QUERY_PLUS(query, "ALL");
+ PQfreemem(q_role);
+ PQfreemem(q_dbname);
+ pfree(query);
+ }
+ }
+
/* ALTER USER,ROLE <name> RESET */
else if (Matches("ALTER", "USER|ROLE", MatchAny, "RESET"))
{
--
2.43.0
Hi,
I got lots of indentation-related warnings when running git apply (see output below).
Also, I found an issue: the RESET command unexpectedly displays "work_mem=16MB",
which is not correct. I've made a minor fix by adding split_part and attached the v3 patch.
```
postgres@zxm-VMware-Virtual-Platform:~/code/postgres$ git apply v2-0001-psql-alter-role-in-database-tab-completion.patch
v2-0001-psql-alter-role-in-database-tab-completion.patch:67: indent with spaces.
/* ALTER ROLE bob IN DATABASE <TAB> → list databases */
v2-0001-psql-alter-role-in-database-tab-completion.patch:68: indent with spaces.
COMPLETE_WITH_QUERY(Query_for_list_of_databases);
v2-0001-psql-alter-role-in-database-tab-completion.patch:73: indent with spaces.
/* ALTER ROLE bob IN DATABASE mydb <TAB> → SET, RESET */
v2-0001-psql-alter-role-in-database-tab-completion.patch:74: indent with spaces.
COMPLETE_WITH("SET", "RESET");
v2-0001-psql-alter-role-in-database-tab-completion.patch:79: indent with spaces.
/* ALTER ROLE bob IN DATABASE mydb SET <TAB> */
warning: squelched 37 whitespace errors
warning: 42 lines add whitespace errors.
```
```
postgres=# ALTER ROLE postgres IN DATABASE postgres SET work_mem = '16MB';
ALTER ROLE
postgres=# ALTER ROLE postgres IN DATABASE postgres RESET
ALL "work_mem=16MB"
```
Could you please take a look and see if this modification is correct?
--
Regards,
Man Zeng
www.openhalo.org
Attachments:
v3-0001-psql-alter-role-in-database-tab-completion.patchapplication/octet-stream; charset=gb18030; name=v3-0001-psql-alter-role-in-database-tab-completion.patchDownload
From 6ad7b789312eef71d066e52062e06cf980052956 Mon Sep 17 00:00:00 2001
From: Man Zeng <zengman@halodbtech.com>
Date: Mon, 29 Dec 2025 15:40:17 +0800
Subject: [PATCH v1] This patch extends psql's tab-completion logic to
recognize the "ALTER ROLE <role> IN DATABASE <dbname>" command structure, and
provides appropriate completions for the SET and RESET subcommands.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Specifically:
• After "ALTER ROLE <role> IN DATABASE <dbname> SET", psql now
completes with the list of configuration variables that may be set
(Query_for_list_of_set_vars), matching the behavior of the plain
ALTER ROLE ... SET form.
• After "ALTER ROLE <role> IN DATABASE <dbname> RESET", psql now
suggests configuration variables that are *actually set* for that
specific (role,database) pair, as recorded in pg_db_role_setting,
plus the keyword ALL. This mirrors the behavior of ALTER DATABASE
... RESET, where we complete only the variables currently set for
the object being modified.
The role name and database name are extracted from the already-parsed
input tokens, and SQL literal quoting is performed via PQescapeLiteral()
using the implicit PGconn (pset.db) available to the tab-completion code.
This avoids any need to alter tab-completion APIs and keeps the patch
self-contained.
Due to the structure of tab-completion, this patch intentionally does
not attempt to complete arbitrary GUC names for RESET, but rather only
those that exist in pg_db_role_setting for the given role and
database. When none are present, psql falls back to suggesting ALL,
matching existing RESET behavior elsewhere.
Vasuki M
---
src/bin/psql/tab-complete.in.c | 65 ++++++++++++++++++++++++++++++++--
1 file changed, 63 insertions(+), 2 deletions(-)
diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c
index 75a101c6ab5..a33818b3654 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -2536,12 +2536,73 @@ match_previous_words(int pattern_id,
else if (Matches("ALTER", "USER|ROLE", MatchAny) &&
!TailMatches("USER", "MAPPING"))
COMPLETE_WITH("BYPASSRLS", "CONNECTION LIMIT", "CREATEDB", "CREATEROLE",
- "ENCRYPTED PASSWORD", "INHERIT", "LOGIN", "NOBYPASSRLS",
- "NOCREATEDB", "NOCREATEROLE", "NOINHERIT",
+ "ENCRYPTED PASSWORD", "IN DATABASE", "INHERIT", "LOGIN",
+ "NOBYPASSRLS", "NOCREATEDB", "NOCREATEROLE", "NOINHERIT",
"NOLOGIN", "NOREPLICATION", "NOSUPERUSER", "PASSWORD",
"RENAME TO", "REPLICATION", "RESET", "SET", "SUPERUSER",
"VALID UNTIL", "WITH");
+ /* ALTER USER,ROLE <name> IN */
+ else if (Matches("ALTER", "USER|ROLE", MatchAny, "IN"))
+ COMPLETE_WITH("DATABASE");
+ /* ALTER USER/ROLE <name> IN DATABASE */
+ else if (Matches("ALTER", "USER|ROLE", MatchAny, "IN", "DATABASE"))
+ {
+ /* ALTER ROLE bob IN DATABASE <TAB> → list databases */
+ COMPLETE_WITH_QUERY(Query_for_list_of_databases);
+ }
+ /* ALTER USER/ROLE <name> IN DATABASE <dbname> */
+ else if (Matches("ALTER", "USER|ROLE", MatchAny, "IN", "DATABASE", MatchAny))
+ {
+ /* ALTER ROLE bob IN DATABASE mydb <TAB> → SET, RESET */
+ COMPLETE_WITH("SET", "RESET");
+ }
+ /* ALTER USER/ROLE <name> IN DATABASE <dbname> SET */
+ else if (Matches("ALTER", "USER|ROLE", MatchAny, "IN", "DATABASE", MatchAny, "SET"))
+ {
+ /* ALTER ROLE bob IN DATABASE mydb SET <TAB> */
+ COMPLETE_WITH_QUERY(Query_for_list_of_set_vars);
+ }
+ /* ALTER USER/ROLE <name> IN DATABASE <dbname> RESET */
+ else if (Matches("ALTER", "USER|ROLE", MatchAny, "IN", "DATABASE", MatchAny, "RESET"))
+ {
+ /*
+ * Extract tokens: prev5 = role name prev2 = database name
+ */
+ char *role = prev5_wd;
+ char *dbname = prev2_wd;
+ char *q_role;
+ char *q_dbname;
+ char *query;
+
+ /* Safe SQL literal quoting using libpq */
+ q_role = PQescapeLiteral(pset.db, role, strlen(role));
+ q_dbname = PQescapeLiteral(pset.db, dbname, strlen(dbname));
+ if (!q_role || !q_dbname)
+ {
+ /* If quoting fails, just fall back to ALL */
+ if (q_role)
+ PQfreemem(q_role);
+ if (q_dbname)
+ PQfreemem(q_dbname);
+ COMPLETE_WITH("ALL");
+ }
+ else
+ {
+ query = psprintf(
+ "SELECT split_part(unnest(setconfig), \'=\', 1) "
+ " FROM pg_db_role_setting "
+ " WHERE setdatabase = "
+ " (SELECT oid FROM pg_database WHERE datname = %s) "
+ " AND setrole = %s::regrole",
+ q_dbname, q_role);
+ COMPLETE_WITH_QUERY_PLUS(query, "ALL");
+ PQfreemem(q_role);
+ PQfreemem(q_dbname);
+ pfree(query);
+ }
+ }
+
/* ALTER USER,ROLE <name> RESET */
else if (Matches("ALTER", "USER|ROLE", MatchAny, "RESET"))
{
--
2.43.0
Hi all,
Thanks everyone for the reviews and discussion — I appreciate the time
spent looking at this.
Based on the feedback, I’ve prepared v4 of the patch with the following
updates:
Added a safety guard before calling PQescapeLiteral() to ensure pset.db is
non-NULL and in CONNECTION_OK state, falling back to ALL otherwise.
Kept the RESET completion limited to parameters actually present in
pg_db_role_setting, matching the behavior of other object-specific RESET
forms.
No changes to tab-completion APIs or libreadline interactions.
Regarding tests: I initially attempted to add TAP coverage via
src/bin/psql/t/010_tab_completion.pl, but based on Tom and Robert’s
comments[on discord], that file is intended to validate libreadline
mechanics rather than individual SQL completion cases. Since this change
does not introduce new readline behavior and SQL-based completion paths
during continuation prompts are not reliably exercised by the current test
harness, I’ve left the patch without TAP coverage.
I’m happy to adjust if there is a preferred place or pattern for testing
this kind of completion in the future.
Thanks again for the guidance.
On Mon, Dec 29, 2025 at 1:18 PM zengman <zengman@halodbtech.com> wrote:
Hi,
I got lots of indentation-related warnings when running git apply (see
output below).
Also, I found an issue: the RESET command unexpectedly displays
"work_mem=16MB",
which is not correct. I've made a minor fix by adding split_part and
attached the v3 patch.Sorry my system is unable to indent with the 2.1.2 version as it is not
yet released,I’ve aligned the indentation with the surrounding code, and CI
/ buildfarm should catch any remaining formatting issues.
Could you please take a look and see if this modification is correct?
Yeah,thanks.
Regards,
Vasuki M
C-DAC,Chennai
Attachments:
v4-0001-psql-alter-role-in-database-tab-completion.patchtext/x-patch; charset=UTF-8; name=v4-0001-psql-alter-role-in-database-tab-completion.patchDownload
From 076fe1e55aed2dc878853723883ee245e8db5451 Mon Sep 17 00:00:00 2001
From: Vasuki M <vasukianand0119@gmail.com>
Date: Tue, 6 Jan 2026 12:07:15 +0530
Subject: [PATCH] [PATCH] psql: Add tab-completion support for ALTER ROLE
... IN DATABASE ... SET/RESET forms This patch extends psql tab
completion to support the ALTER ROLE <role> IN DATABASE <dbname> command
form.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
After SET, psql completes with the list of configurable GUC variables,
matching the behavior of ALTER ROLE … SET.
After RESET, psql completes with configuration variables actually set
for the given (role, database) pair as recorded in
pg_db_role_setting, plus ALL, mirroring ALTER DATABASE … RESET
behavior.
SQL literal quoting is performed using PQescapeLiteral() via the
implicit connection (pset.db). A safety guard ensures a valid
connection is present before invoking it, falling back to ALL
otherwise.
No TAP tests are added, as this change does not introduce new
libreadline interactions and SQL-based completion paths are not
reliably exercised by the current test harness.
Vasuki M
---
src/bin/psql/tab-complete.in.c | 66 +++++++++++++++++++---------------
1 file changed, 37 insertions(+), 29 deletions(-)
diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c
index 5174d025531..c5fc2354797 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -2566,40 +2566,48 @@ match_previous_words(int pattern_id,
/* ALTER USER/ROLE <name> IN DATABASE <dbname> RESET */
else if (Matches("ALTER", "USER|ROLE", MatchAny, "IN", "DATABASE", MatchAny, "RESET"))
{
- /*
- * Extract tokens: prev5 = role name prev2 = database name
- */
- char *role = prev5_wd;
- char *dbname = prev2_wd;
- char *q_role;
- char *q_dbname;
- char *query;
-
- /* Safe SQL literal quoting using libpq */
- q_role = PQescapeLiteral(pset.db, role, strlen(role));
- q_dbname = PQescapeLiteral(pset.db, dbname, strlen(dbname));
- if (!q_role || !q_dbname)
+ if (!pset.db || PQstatus(pset.db) != CONNECTION_OK)
{
- /* If quoting fails, just fall back to ALL */
- if (q_role)
- PQfreemem(q_role);
- if (q_dbname)
- PQfreemem(q_dbname);
COMPLETE_WITH("ALL");
}
else
{
- query = psprintf(
- " SELECT split_part(unnest(setconfig), \'=\', 1) "
- " FROM pg_db_role_setting "
- " WHERE setdatabase = "
- " (SELECT oid FROM pg_database WHERE datname = %s) "
- " AND setrole = %s::regrole",
- q_dbname, q_role);
- COMPLETE_WITH_QUERY_PLUS(query, "ALL");
- PQfreemem(q_role);
- PQfreemem(q_dbname);
- pfree(query);
+ /*
+ * Extract tokens: prev5 = role name prev2 = database name
+ */
+ char *role = prev5_wd;
+ char *dbname = prev2_wd;
+ char *q_role;
+ char *q_dbname;
+ char *query;
+
+ /* Safe SQL literal quoting using libpq */
+ q_role = PQescapeLiteral(pset.db, role, strlen(role));
+ q_dbname = PQescapeLiteral(pset.db, dbname, strlen(dbname));
+ if (!q_role || !q_dbname)
+ {
+ /* If quoting fails, just fall back to ALL */
+ if (q_role)
+ PQfreemem(q_role);
+ if (q_dbname)
+ PQfreemem(q_dbname);
+ COMPLETE_WITH("ALL");
+ }
+ else
+ {
+ query = psprintf(
+ " SELECT split_part(unnest(setconfig), \'=\', 1) "
+ " FROM pg_db_role_setting "
+ " WHERE setdatabase = "
+ " (SELECT oid FROM pg_database WHERE datname = %s) "
+ " AND setrole = %s::regrole",
+ q_dbname, q_role);
+ COMPLETE_WITH_QUERY_PLUS(query, "ALL");
+ PQfreemem(q_role);
+ PQfreemem(q_dbname);
+ pfree(query);
+ }
+
}
}
--
2.43.0
Hi,
This v4 patch appears incomplete; my `git apply` command failed.
CFbot: Patch needs rebase
--
Regards,
Man Zeng
www.openhalo.org
Sorry,it was an incremental patch.
The attached v5 is the fresh patch against Master.
Regards,
Vasuki M
C-DAC,Chennai
Attachments:
v5-0001-psql-alter-role-in-database-tab-completion.patchtext/x-patch; charset=UTF-8; name=v5-0001-psql-alter-role-in-database-tab-completion.patchDownload
From 57e7f3ce7397467466ae705dbe29c783de87e310 Mon Sep 17 00:00:00 2001
From: Vasuki M <vasukianand0119@gmail.com>
Date: Tue, 6 Jan 2026 14:53:33 +0530
Subject: [PATCH] psql: Add tab-completion support for ALTER ROLE ... IN
DATABASE ... SET/RESET forms .This patch extends psql tab completion to
support the ALTER ROLE <role> IN DATABASE <dbname> command form.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
After SET, psql completes with the list of configurable GUC variables,
matching the behavior of ALTER ROLE … SET.
After RESET, psql completes with configuration variables actually set
for the given (role, database) pair as recorded in
pg_db_role_setting, plus ALL, mirroring ALTER DATABASE … RESET
behavior.
SQL literal quoting is performed using PQescapeLiteral() via the
implicit connection (pset.db). A safety guard ensures a valid
connection is present before invoking it, falling back to ALL
otherwise.
No TAP tests are added, as this change does not introduce new
libreadline interactions and SQL-based completion paths are not
reliably exercised by the current test harness.
Vasuki M
---
src/bin/psql/tab-complete.in.c | 70 +++++++++++++++++++++++++++++++++-
1 file changed, 68 insertions(+), 2 deletions(-)
diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c
index 06edea98f06..3dd46fddb57 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -2536,12 +2536,78 @@ match_previous_words(int pattern_id,
else if (Matches("ALTER", "USER|ROLE", MatchAny) &&
!TailMatches("USER", "MAPPING"))
COMPLETE_WITH("BYPASSRLS", "CONNECTION LIMIT", "CREATEDB", "CREATEROLE",
- "ENCRYPTED PASSWORD", "INHERIT", "LOGIN", "NOBYPASSRLS",
+ "ENCRYPTED PASSWORD", "INHERIT","IN DATABASE", "LOGIN", "NOBYPASSRLS",
"NOCREATEDB", "NOCREATEROLE", "NOINHERIT",
"NOLOGIN", "NOREPLICATION", "NOSUPERUSER", "PASSWORD",
"RENAME TO", "REPLICATION", "RESET", "SET", "SUPERUSER",
"VALID UNTIL", "WITH");
-
+ /* ALTER USER,ROLE <name> IN */
+ else if (Matches("ALTER", "USER|ROLE", MatchAny, "IN"))
+ COMPLETE_WITH("DATABASE");
+ /* ALTER USER/ROLE <name> IN DATABASE */
+ else if (Matches("ALTER", "USER|ROLE", MatchAny, "IN", "DATABASE"))
+ {
+ /* ALTER ROLE bob IN DATABASE <TAB> → list databases */
+ COMPLETE_WITH_QUERY(Query_for_list_of_databases);
+ }
+ /* ALTER USER/ROLE <name> IN DATABASE <dbname> */
+ else if (Matches("ALTER", "USER|ROLE", MatchAny, "IN", "DATABASE", MatchAny))
+ {
+ /* ALTER ROLE bob IN DATABASE mydb <TAB> → SET, RESET */
+ COMPLETE_WITH("SET", "RESET");
+ }
+ /* ALTER USER/ROLE <name> IN DATABASE <dbname> SET */
+ else if (Matches("ALTER", "USER|ROLE", MatchAny, "IN", "DATABASE", MatchAny, "SET"))
+ {
+ /* ALTER ROLE bob IN DATABASE mydb SET <TAB> */
+ COMPLETE_WITH_QUERY(Query_for_list_of_set_vars);
+ }
+ /* ALTER USER/ROLE <name> IN DATABASE <dbname> RESET */
+ else if (Matches("ALTER", "USER|ROLE", MatchAny, "IN", "DATABASE", MatchAny, "RESET"))
+ {
+ if (!pset.db || PQstatus(pset.db) != CONNECTION_OK)
+ {
+ COMPLETE_WITH("ALL");
+ }
+ else
+ {
+ /*
+ * Extract tokens: prev5 = role name prev2 = database name
+ */
+ char *role = prev5_wd;
+ char *dbname = prev2_wd;
+ char *q_role;
+ char *q_dbname;
+ char *query;
+
+ /* Safe SQL literal quoting using libpq */
+ q_role = PQescapeLiteral(pset.db, role, strlen(role));
+ q_dbname = PQescapeLiteral(pset.db, dbname, strlen(dbname));
+ if (!q_role || !q_dbname)
+ {
+ /* If quoting fails, just fall back to ALL */
+ if (q_role)
+ PQfreemem(q_role);
+ if (q_dbname)
+ PQfreemem(q_dbname);
+ COMPLETE_WITH("ALL");
+ }
+ else
+ {
+ query = psprintf(
+ " SELECT split_part(unnest(setconfig), \'=\', 1) "
+ " FROM pg_db_role_setting "
+ " WHERE setdatabase = "
+ " (SELECT oid FROM pg_database WHERE datname = %s) "
+ " AND setrole = %s::regrole",
+ q_dbname, q_role);
+ COMPLETE_WITH_QUERY_PLUS(query, "ALL");
+ PQfreemem(q_role);
+ PQfreemem(q_dbname);
+ pfree(query);
+ }
+ }
+ }
/* ALTER USER,ROLE <name> RESET */
else if (Matches("ALTER", "USER|ROLE", MatchAny, "RESET"))
{
--
2.43.0
Added a safety guard before calling PQescapeLiteral() to ensure pset.db is non-NULL and in CONNECTION_OK state, falling back to ALL otherwise.
Hi, VASUKI M
I just looked at this patch and thought about it briefly, but I'm a little confused about the purpose of this conditional check.
If the condition is met, then returning anything at this point makes no sense, right?
```
if (!pset.db || PQstatus(pset.db) != CONNECTION_OK)
{
COMPLETE_WITH("ALL");
}
```
Furthermore, I believe this should be guaranteed by the function `exec_query`.
```
COMPLETE_WITH_QUERY_PLUS -> COMPLETE_WITH_QUERY_LIST -> complete_from_query -> exec_query
```
However, I haven't had time to debug it yet, so I might need your help to double-check it.
--
Regards,
Man Zeng
www.openhalo.org
Hi,
Thanks for taking a closer look — that’s a fair question.
The guard is not meant to protect exec_query(), but rather the calls to
PQescapeLiteral(), which are executed before COMPLETE_WITH_QUERY_PLUS
and therefore before exec_query() is reached.
In this code path we construct the query string by calling
PQescapeLiteral(pset.db, ...) directly, and that function assumes a
non-NULL, valid PGconn. If pset.db is NULL or the connection is not in
CONNECTION_OK state, calling PQescapeLiteral() would be unsafe and could
lead to a crash before exec_query() has a chance to handle anything.
The fallback to COMPLETE_WITH("ALL") is intended as a safe default, matching
existing RESET completion behavior when no catalog information can be
retrieved.
That said, I agree it’s worth double-checking whether pset.db is always
guaranteed to be valid in this context, and I’m happy to refine or document
this further if you think a different approach would be better.
Thanks for flagging this .
Regards,
Vasuki M
C-DAC,Chennai
Hello,
That guard was actually my suggestion in review, and the reason is that
it’s protecting the PQescapeLiteral() calls, not exec_query().
In the RESET completion branch we call PQescapeLiteral(pset.db, ...) to
safely quote the role/dbname before we even build the SQL string. That
happens before COMPLETE_WITH_QUERY_PLUS ever reaches complete_from_query()
/ exec_query(). So while exec_query() does guard query execution when
pset.db is null or not CONNECTION_OK, it doesn’t prevent us from passing a
null conn into PQescapeLiteral(), which could crash or otherwise misbehave.
If we don’t have a usable connection, we can’t reliably quote and run the
catalog query anyway, so falling back to a static completion like ALL seems
like the safest behavior. Hence I think it's valid.
Regarding tests: I initially attempted to add TAP coverage via
src/bin/psql/t/010_tab_completion.pl, but based on Tom and Robert’s
comments[on discord], that file is intended to validate libreadline
mechanics rather than individual SQL completion cases. Since this change
does not introduce new readline behavior and SQL-based completion paths
during continuation prompts are not reliably exercised by the current test
harness, I’ve left the patch without TAP coverage.
On testing: I did try adding TAP coverage in src/bin/psql/t/
010_tab_completion.pl, and we can in fact cover most of this feature there
today.
In particular, 010_tab_completion.pl already contains a number of
“individual completion case” checks beyond pure readline plumbing (e.g.,
query-driven completions like schema-qualified object references, timezone
completion, COPY DEFAULT, etc.), so adding targeted cases for ALTER ROLE
... IN DATABASE seems consistent with existing practice.
What we were able to cover reliably:
- keyword progression (IN DATABASE)
- database name completion after IN DATABASE postgres
- offering SET/RESET after the database name (<TAB><TAB>)
- SET keyword completion and GUC name completion (SET work_<TAB> ->
work_mem)
What I could not make reliable in TAP is the query-driven RESET variable
listing itself. The new completion rule only triggers at “… RESET <TAB>”
(cursor immediately after RESET), so prefix insertion tests like “RESET
wo<TAB>” won’t exercise it. That leaves list-style output (“RESET
<TAB><TAB>”), which is both highly variant across readline/libedit
implementations and not reliably matchable with the current
query_until()-based harness, leading to timeouts/flakiness even though
manual interactive testing confirms it works.
Given that, I think keeping the existing TAP coverage for the deterministic
parts (as above) plus a short comment in 010 explaining why the RESET
variable-listing output isn’t asserted is a reasonable compromise. If
there’s a preferred pattern/harness improvement to make query-driven list
output stable, I’m happy to follow that direction.
Thanks,
Dharin
On Tue, Jan 6, 2026 at 11:54 AM VASUKI M <vasukianand0119@gmail.com> wrote:
Show quoted text
Hi,
Thanks for taking a closer look — that’s a fair question.
The guard is not meant to protect exec_query(), but rather the calls to
PQescapeLiteral(), which are executed before COMPLETE_WITH_QUERY_PLUS
and therefore before exec_query() is reached.In this code path we construct the query string by calling
PQescapeLiteral(pset.db, ...) directly, and that function assumes a
non-NULL, valid PGconn. If pset.db is NULL or the connection is not in
CONNECTION_OK state, calling PQescapeLiteral() would be unsafe and could
lead to a crash before exec_query() has a chance to handle anything.The fallback to COMPLETE_WITH("ALL") is intended as a safe default,
matching
existing RESET completion behavior when no catalog information can be
retrieved.That said, I agree it’s worth double-checking whether pset.db is always
guaranteed to be valid in this context, and I’m happy to refine or document
this further if you think a different approach would be better.Thanks for flagging this .
Regards,
Vasuki M
C-DAC,Chennai
That guard was actually my suggestion in review, and the reason is that it’s protecting the PQescapeLiteral() calls, not exec_query().
In the RESET completion branch we call PQescapeLiteral(pset.db, ...) to safely quote the role/dbname before we even build the SQL string. That happens before COMPLETE_WITH_QUERY_PLUS ever reaches complete_from_query() / exec_query(). So while exec_query() does guard query execution when pset.db is null or not CONNECTION_OK, it doesn’t p> revent us from passing a null conn into PQescapeLiteral(), which could crash or otherwise misbehave.
If we don’t have a usable connection, we can’t reliably quote and run the catalog query anyway, so falling back to a static completion like ALL seems like the safest behavior. Hence I think it's valid.
Hi,
Thank you both for clarifying my confusion. I hadn't paid much attention to `PQescapeLiteral` earlier.
I checked the function, and it should return NULL on failure.
```
q_role = PQescapeLiteral(pset.db, role, strlen(role));
q_dbname = PQescapeLiteral(pset.db, dbname, strlen(dbname));
if (!q_role || !q_dbname)
{
/* If quoting fails, just fall back to ALL */
if (q_role)
PQfreemem(q_role);
if (q_dbname)
PQfreemem(q_dbname);
COMPLETE_WITH("ALL");
}
```
When `pset.db` is NULL or for other reasons (resulting in q_role/q_dbname being NULL), `!q_role || !q_dbname` will be hit — this already meets our needs.
I wonder if this understanding is correct — what do you think?
--
Regards,
Man Zeng
www.openhalo.org
Hi Man,
Your reading of the local logic is totally reasonable and correct: if
PQescapeLiteral() returns NULL for any reason, the existing if (!q_role ||
!q_dbname) path will indeed fall back to ALL, so quoting failures are
already handled.
The only thing I’m cautious about is treating “pset.db is NULL/invalid” as
just another “quoting failure” case. In this completion branch we call
PQescapeLiteral(pset.db, ...) before we ever reach exec_query(), so an
explicit guard is about avoiding passing an unusable handle into libpq in
the first place. Even if libpq were to return NULL in that situation, it’s
not something I’d want to rely on implicitly.
That’s why I suggested the explicit guard: it matches the general psql
style of checking !pset.db before calling libpq APIs (e.g.
psql_get_variable() in src/bin/psql/common.c checks !pset.db before calling
PQescapeLiteral()), and it makes the intent obviously safe. Behavior-wise
it’s the same (fall back to ALL), just more defensive/clear & explicit.
Thanks,
Dharin
On Tue, Jan 6, 2026 at 2:34 PM zengman <zengman@halodbtech.com> wrote:
Show quoted text
That guard was actually my suggestion in review, and the reason is that
it’s protecting the PQescapeLiteral() calls, not exec_query().
In the RESET completion branch we call PQescapeLiteral(pset.db, ...) to
safely quote the role/dbname before we even build the SQL string. That
happens before COMPLETE_WITH_QUERY_PLUS ever reaches complete_from_query()
/ exec_query(). So while exec_query() does guard query execution when
pset.db is null or not CONNECTION_OK, it doesn’t p> revent us from passing
a null conn into PQescapeLiteral(), which could crash or otherwise
misbehave.If we don’t have a usable connection, we can’t reliably quote and run
the catalog query anyway, so falling back to a static completion like ALL
seems like the safest behavior. Hence I think it's valid.Hi,
Thank you both for clarifying my confusion. I hadn't paid much attention
to `PQescapeLiteral` earlier.
I checked the function, and it should return NULL on failure.
```
q_role = PQescapeLiteral(pset.db, role, strlen(role));
q_dbname = PQescapeLiteral(pset.db, dbname, strlen(dbname));
if (!q_role || !q_dbname)
{
/* If quoting fails, just fall back to ALL */
if (q_role)
PQfreemem(q_role);
if (q_dbname)
PQfreemem(q_dbname);
COMPLETE_WITH("ALL");
}
```
When `pset.db` is NULL or for other reasons (resulting in q_role/q_dbname
being NULL), `!q_role || !q_dbname` will be hit — this already meets our
needs.
I wonder if this understanding is correct — what do you think?--
Regards,
Man Zeng
www.openhalo.org
The only thing I’m cautious about is treating “pset.db is NULL/invalid” as just another “quoting failure” case. In this completion branch we call PQescapeLiteral(pset.db, ...) before we ever reach exec_query(), so an explicit guard is about avoiding passing an unusable handle into libpq in the first place. Even if libpq were to return NULL in that situation, it’s > not something I’d want to rely on implicitly.
That’s why I suggested the explicit guard: it matches the general psql style of checking !pset.db before calling libpq APIs (e.g. psql_get_variable() in src/bin/psql/common.c checks !pset.db before calling PQescapeLiteral()), and it makes the intent obviously safe. Behavior-wise it’s the same (fall back to ALL), just more defensive/clear & explicit.
Hi,
Okay, I understand what you mean, thank you.
--
Regards,
Man Zeng
www.openhalo.org
Thank you Dharin, Man Zeng for the great comments.
I feel Vasuki's latest patch is in good shape.
On Tue, Jan 6, 2026 at 5:55 AM zengman <zengman@halodbtech.com> wrote:
Show quoted text
The only thing I’m cautious about is treating “pset.db is NULL/invalid”
as just another “quoting failure” case. In this completion branch we call
PQescapeLiteral(pset.db, ...) before we ever reach exec_query(), so an
explicit guard is about avoiding passing an unusable handle into libpq in
the first place. Even if libpq were to return NULL in that situation, it’snot something I’d want to rely on implicitly.
That’s why I suggested the explicit guard: it matches the general psqlstyle of checking !pset.db before calling libpq APIs (e.g.
psql_get_variable() in src/bin/psql/common.c checks !pset.db before calling
PQescapeLiteral()), and it makes the intent obviously safe. Behavior-wise
it’s the same (fall back to ALL), just more defensive/clear & explicit.
Hi,Okay, I understand what you mean, thank you.
--
Regards,
Man Zeng
www.openhalo.org
Hii,
On Tue, Jan 6, 2026 at 5:49 PM Dharin Shah <dharinshah95@gmail.com> wrote:
What we were able to cover reliably:
- keyword progression (IN DATABASE)
- database name completion after IN DATABASE postgres
- offering SET/RESET after the database name (<TAB><TAB>)
- SET keyword completion and GUC name completion (SET work_<TAB> ->
work_mem)What I could not make reliable in TAP is the query-driven RESET variable
listing itself. The new completion rule only triggers at “… RESET <TAB>”
(cursor immediately after RESET), so prefix insertion tests like “RESET
wo<TAB>” won’t exercise it. That leaves list-style output (“RESET
<TAB><TAB>”), which is both highly variant across readline/libedit
implementations and not reliably matchable with the current
query_until()-based harness, leading to timeouts/flakiness even though
manual interactive testing confirms it works.Given that, I think keeping the existing TAP coverage for the
deterministic parts (as above) plus a short comment in 010 explaining why
the RESET variable-listing output isn’t asserted is a reasonable
compromise. If there’s a preferred pattern/harness improvement to make
query-driven list output stable, I’m happy to follow that direction.
I reached the same conclusion while experimenting with TAP coverage.
The query-driven RESET variable listing only triggers at the exact
… RESET <TAB> position, and does not support prefix-based matching
(e.g. RESET wo<TAB>). This leaves only list-style output
(RESET <TAB><TAB>), which is highly dependent on the readline/libedit
implementation and does not behave deterministically under the current
query_until()-based test harness. In practice this leads to timeouts
or flakiness, even though the behavior works correctly in interactive
psql sessions.
Given this, I agree that adding partial or unreliable TAP assertions
would be worse than omitting them. Keeping coverage for the deterministic
parts (keyword progression, database name completion, SET behavior) and
documenting why RESET variable listing is not asserted seems like the
most reasonable approach for now.
If, in the future, the TAP harness is extended to better support
query-driven list output during completion, this would be a good area
to revisit. I’d be happy to help explore that direction when such a
framework exists.
This patch added in commitfest https://commitfest.postgresql.org/patch/6244/
Thanks,
Vasuki M
C-DAC,Chennai