Fix tab completion in v18 for ALTER DATABASE/USER/ROLE ... RESET
Hi hackers,
These two patches are split out from my earlier thread about improving
tab completion for varous RESET forms
(/messages/by-id/87bjqwwtic.fsf@wibble.ilmari.org), so that the bug
fixes can be tracked as an open item for v18.
Commits 9df8727c5067 and c407d5426b87 added tab completion for ALTER
DATABASE ... RESET and ALTER ROLE/USER ... RESET, respectively, but
they're both differently buggy. The query for the DATABASE form
neglected to schema-qualify the unnest() call, and the query for the
ROLE/USER form shows all possible variables once you start typing a
variable name, not just the ones that are set on the role. The attached
patches fix both.
- ilmari
Attachments:
0001-Schema-qualify-unnest-call-in-ALTER-DATABASE-.-RESET.patchtext/x-diffDownload
From 0dc6058941b5195b58d26ad5baeebc9fb4ca36fb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Wed, 11 Jun 2025 13:10:54 +0100
Subject: [PATCH 1/2] Schema-qualify unnest() call in ALTER DATABASE ... RESET
tab completion
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 37524364290..5ca9bc2fc0d 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -1010,7 +1010,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.50.1
0002-Fix-tab-completion-for-ALTER-ROLE-USER-.-RESET.patchtext/x-diffDownload
From 83bf13c9ea813047d1c462434fe32c54db9b28dc 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:35:29 +0100
Subject: [PATCH 2/2] Fix tab completion for ALTER ROLE|USER ... RESET
Commit c407d5426b87 added tab completion for ALTER ROLE|USER
... RESET, but it was buggy.
It showed only the variables set on the role when hitting TAB just
after RESET, but as soon as one starts typing something to pick what
to complete, it would complete all possible variables, not just the
ones set on the role. 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 5ca9bc2fc0d..257ae55ee32 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -1086,9 +1086,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 "\
@@ -2516,7 +2519,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.50.1
On Thu, Jul 17, 2025 at 1:41 AM Dagfinn Ilmari Mannsåker
<ilmari@ilmari.org> wrote:
Hi hackers,
These two patches are split out from my earlier thread about improving
tab completion for varous RESET forms
(/messages/by-id/87bjqwwtic.fsf@wibble.ilmari.org), so that the bug
fixes can be tracked as an open item for v18.Commits 9df8727c5067 and c407d5426b87 added tab completion for ALTER
DATABASE ... RESET and ALTER ROLE/USER ... RESET, respectively, but
they're both differently buggy. The query for the DATABASE form
neglected to schema-qualify the unnest() call, and the query for the
ROLE/USER form shows all possible variables once you start typing a
variable name, not just the ones that are set on the role. The attached
patches fix both.
I played around with it, and overall it looks good.
jian he <jian.universality@gmail.com> writes:
On Thu, Jul 17, 2025 at 1:41 AM Dagfinn Ilmari Mannsåker
<ilmari@ilmari.org> wrote:Hi hackers,
These two patches are split out from my earlier thread about improving
tab completion for varous RESET forms
(/messages/by-id/87bjqwwtic.fsf@wibble.ilmari.org), so that the bug
fixes can be tracked as an open item for v18.Commits 9df8727c5067 and c407d5426b87 added tab completion for ALTER
DATABASE ... RESET and ALTER ROLE/USER ... RESET, respectively, but
they're both differently buggy. The query for the DATABASE form
neglected to schema-qualify the unnest() call, and the query for the
ROLE/USER form shows all possible variables once you start typing a
variable name, not just the ones that are set on the role. The attached
patches fix both.I played around with it, and overall it looks good.
Thanks for the review.
I realise I should have included Robins and Tomas in the original mail,
since they were the author and committer, respectively, of the original
patches. I've added them now, and reattached the patches for their
convenience.
- ilmari
Attachments:
0001-Schema-qualify-unnest-call-in-ALTER-DATABASE-.-RESET.patchtext/x-diffDownload
From 0dc6058941b5195b58d26ad5baeebc9fb4ca36fb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Wed, 11 Jun 2025 13:10:54 +0100
Subject: [PATCH 1/2] Schema-qualify unnest() call in ALTER DATABASE ... RESET
tab completion
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 37524364290..5ca9bc2fc0d 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -1010,7 +1010,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.50.1
0002-Fix-tab-completion-for-ALTER-ROLE-USER-.-RESET.patchtext/x-diffDownload
From 83bf13c9ea813047d1c462434fe32c54db9b28dc 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:35:29 +0100
Subject: [PATCH 2/2] Fix tab completion for ALTER ROLE|USER ... RESET
Commit c407d5426b87 added tab completion for ALTER ROLE|USER
... RESET, but it was buggy.
It showed only the variables set on the role when hitting TAB just
after RESET, but as soon as one starts typing something to pick what
to complete, it would complete all possible variables, not just the
ones set on the role. 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 5ca9bc2fc0d..257ae55ee32 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -1086,9 +1086,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 "\
@@ -2516,7 +2519,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.50.1
On 7/22/25 13:18, Dagfinn Ilmari Mannsåker wrote:
jian he <jian.universality@gmail.com> writes:
On Thu, Jul 17, 2025 at 1:41 AM Dagfinn Ilmari Mannsåker
<ilmari@ilmari.org> wrote:Hi hackers,
These two patches are split out from my earlier thread about improving
tab completion for varous RESET forms
(/messages/by-id/87bjqwwtic.fsf@wibble.ilmari.org), so that the bug
fixes can be tracked as an open item for v18.Commits 9df8727c5067 and c407d5426b87 added tab completion for ALTER
DATABASE ... RESET and ALTER ROLE/USER ... RESET, respectively, but
they're both differently buggy. The query for the DATABASE form
neglected to schema-qualify the unnest() call, and the query for the
ROLE/USER form shows all possible variables once you start typing a
variable name, not just the ones that are set on the role. The attached
patches fix both.I played around with it, and overall it looks good.
Thanks for the review.
I realise I should have included Robins and Tomas in the original mail,
since they were the author and committer, respectively, of the original
patches. I've added them now, and reattached the patches for their
convenience.
Thanks for the fixes. Both seem correct to me. It took me a while to
reproduce some sort of issue with unnest(), but that was mostly because
I didn't realize the tab completion does not report errors to the user.
While testing the ALTER ROLE part, I realized there's a second related
issue with similar symptoms - after starting to type a variable that is
*not* set for the role, it was offering all matching variables anyway. I
believe that's because of the block at line ~5022. The "database" case
was already excluded, so I made 0002 to do that for ROLE too.
I plan to push the attached fixes soon ...
regards
--
Tomas Vondra
Attachments:
v2-0002-Fix-tab-completion-for-ALTER-ROLE-USER-.-RESET.patchtext/x-patch; charset=UTF-8; name=v2-0002-Fix-tab-completion-for-ALTER-ROLE-USER-.-RESET.patchDownload
From fa26b62298d7a4221d9bcefe71767d337842cabc 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:35:29 +0100
Subject: [PATCH v2 2/2] Fix tab completion for ALTER ROLE|USER ... RESET
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Commit c407d5426b87 added tab completion for ALTER ROLE|USER ... RESET,
with the intent to offer only the variables actually set on the role.
But as soon as the user started typing something, it would start to
offer all possible matching variables.
Fix this the same way ALTER DATABASE ... RESET does it, i.e. by
properly considering the prefix.
A second issue causing similar symptoms (offering variables that are not
actually set for a role) was caused by a match to another pattern. The
ALTER DATABASE ... RESET was already excluded, so do the same thing for
ROLE/USER.
Report and fix by Dagfinn Ilmari Mannsåker. Backpatch to 18, same as
c407d5426b87.
Author: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Reviewed-by: jian he <jian.universality@gmail.com>
Discussion: https://postgr.es/m/87qzyghw2x.fsf%40wibble.ilmari.org
Discussion: https://postgr.es/m/87tt4lumqz.fsf%40wibble.ilmari.org
---
src/bin/psql/tab-complete.in.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c
index 3c50847f958..ba5f5861221 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -1086,9 +1086,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 "\
@@ -2517,7 +2520,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"))
@@ -5015,7 +5021,8 @@ match_previous_words(int pattern_id,
/* Complete with a variable name */
else if (TailMatches("SET|RESET") &&
!TailMatches("UPDATE", MatchAny, "SET") &&
- !TailMatches("ALTER", "DATABASE", MatchAny, "RESET"))
+ !TailMatches("ALTER", "DATABASE", MatchAny, "RESET") &&
+ !TailMatches("ALTER", "USER|ROLE", MatchAny, "RESET"))
COMPLETE_WITH_QUERY_VERBATIM_PLUS(Query_for_list_of_set_vars,
"CONSTRAINTS",
"TRANSACTION",
--
2.50.1
v2-0001-Schema-qualify-unnest-in-ALTER-DATABASE-.-RESET.patchtext/x-patch; charset=UTF-8; name=v2-0001-Schema-qualify-unnest-in-ALTER-DATABASE-.-RESET.patchDownload
From f97c82851b7a8f74d0b3ab633326283a93be0823 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Wed, 11 Jun 2025 13:10:54 +0100
Subject: [PATCH v2 1/2] Schema-qualify unnest() in ALTER DATABASE ... RESET
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Commit 9df8727c5067 failed to schema-quality the unnest() call in the
query used to list the variables in ALTER DATABASE ... RESET. If there's
another unnest() function in the search_path, this could cause either
failures, or even security issues (when the tab-completion gets used by
privileged accounts).
Report and fix by Dagfinn Ilmari Mannsåker. Backpatch to 18, same as
9df8727c5067.
Author: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Reviewed-by: jian he <jian.universality@gmail.com>
Discussion: https://postgr.es/m/87qzyghw2x.fsf%40wibble.ilmari.org
Discussion: https://postgr.es/m/87tt4lumqz.fsf%40wibble.ilmari.org
---
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 dbc586c5bc3..3c50847f958 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -1010,7 +1010,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.50.1
Tomas Vondra <tomas@vondra.me> writes:
On 7/22/25 13:18, Dagfinn Ilmari Mannsåker wrote:
jian he <jian.universality@gmail.com> writes:
On Thu, Jul 17, 2025 at 1:41 AM Dagfinn Ilmari Mannsåker
<ilmari@ilmari.org> wrote:Hi hackers,
These two patches are split out from my earlier thread about improving
tab completion for varous RESET forms
(/messages/by-id/87bjqwwtic.fsf@wibble.ilmari.org), so that the bug
fixes can be tracked as an open item for v18.Commits 9df8727c5067 and c407d5426b87 added tab completion for ALTER
DATABASE ... RESET and ALTER ROLE/USER ... RESET, respectively, but
they're both differently buggy. The query for the DATABASE form
neglected to schema-qualify the unnest() call, and the query for the
ROLE/USER form shows all possible variables once you start typing a
variable name, not just the ones that are set on the role. The attached
patches fix both.I played around with it, and overall it looks good.
Thanks for the review.
I realise I should have included Robins and Tomas in the original mail,
since they were the author and committer, respectively, of the original
patches. I've added them now, and reattached the patches for their
convenience.Thanks for the fixes. Both seem correct to me. It took me a while to
reproduce some sort of issue with unnest(), but that was mostly because
I didn't realize the tab completion does not report errors to the user.While testing the ALTER ROLE part, I realized there's a second related
issue with similar symptoms - after starting to type a variable that is
*not* set for the role, it was offering all matching variables anyway. I
believe that's because of the block at line ~5022. The "database" case
was already excluded, so I made 0002 to do that for ROLE too.
Ah, good catch, I missed that edge case.
@@ -5015,7 +5021,8 @@ match_previous_words(int pattern_id, /* Complete with a variable name */ else if (TailMatches("SET|RESET") && !TailMatches("UPDATE", MatchAny, "SET") && - !TailMatches("ALTER", "DATABASE", MatchAny, "RESET")) + !TailMatches("ALTER", "DATABASE", MatchAny, "RESET") && + !TailMatches("ALTER", "USER|ROLE", MatchAny, "RESET")) COMPLETE_WITH_QUERY_VERBATIM_PLUS(Query_for_list_of_set_vars, "CONSTRAINTS", "TRANSACTION",
Instead of adding another !TailMatches() call, why not just change
"DATABASE" to "DATABASE|ROLE|USER"?
- ilmari
On 7/30/25 12:17, Dagfinn Ilmari Mannsåker wrote:
...
@@ -5015,7 +5021,8 @@ match_previous_words(int pattern_id, /* Complete with a variable name */ else if (TailMatches("SET|RESET") && !TailMatches("UPDATE", MatchAny, "SET") && - !TailMatches("ALTER", "DATABASE", MatchAny, "RESET")) + !TailMatches("ALTER", "DATABASE", MatchAny, "RESET") && + !TailMatches("ALTER", "USER|ROLE", MatchAny, "RESET")) COMPLETE_WITH_QUERY_VERBATIM_PLUS(Query_for_list_of_set_vars, "CONSTRAINTS", "TRANSACTION",Instead of adding another !TailMatches() call, why not just change
"DATABASE" to "DATABASE|ROLE|USER"?
It seemed to me separate calls would be easier to understand, but I see
combine it like this in many other places, so done that way ...
Pushed. Thanks for the fixes!
regards
--
Tomas Vondra
Tomas Vondra <tomas@vondra.me> writes:
On 7/30/25 12:17, Dagfinn Ilmari Mannsåker wrote:
...
@@ -5015,7 +5021,8 @@ match_previous_words(int pattern_id, /* Complete with a variable name */ else if (TailMatches("SET|RESET") && !TailMatches("UPDATE", MatchAny, "SET") && - !TailMatches("ALTER", "DATABASE", MatchAny, "RESET")) + !TailMatches("ALTER", "DATABASE", MatchAny, "RESET") && + !TailMatches("ALTER", "USER|ROLE", MatchAny, "RESET")) COMPLETE_WITH_QUERY_VERBATIM_PLUS(Query_for_list_of_set_vars, "CONSTRAINTS", "TRANSACTION",Instead of adding another !TailMatches() call, why not just change
"DATABASE" to "DATABASE|ROLE|USER"?It seemed to me separate calls would be easier to understand, but I see
combine it like this in many other places, so done that way ...Pushed.
Thanks!
Show quoted text
Thanks for the fixes!
regards