Feedback-ID: i1ff147bf:Fastmail
From: =?utf-8?Q?Dagfinn_Ilmari_Manns=C3=A5ker?= <ilmari@ilmari.org>
To: jian he <jian.universality@gmail.com>, Robins Tharakan
 <tharakan@gmail.com>, Tomas Vondra <tomas.vondra@postgresql.org>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Fix tab completion in v18 for ALTER DATABASE/USER/ROLE ... RESET
In-Reply-To: 
 <CACJufxF5_23CykxYOFYeXMR6_XC+fjjUmY5NRf=JQDWR26q4XA@mail.gmail.com>
	(jian he's message of "Tue, 22 Jul 2025 18:28:18 +0800")
References: <87qzyghw2x.fsf@wibble.ilmari.org>
	<CACJufxF5_23CykxYOFYeXMR6_XC+fjjUmY5NRf=JQDWR26q4XA@mail.gmail.com>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux)
Date: Tue, 22 Jul 2025 12:18:28 +0100
Message-ID: <87ecu8iid7.fsf@wibble.ilmari.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="=-=-="
Archived-At: 
 <https://www.postgresql.org/message-id/87ecu8iid7.fsf%40wibble.ilmari.org>
Precedence: bulk

--=-=-=
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit

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
>> (https://postgr.es/m/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


--=-=-=
Content-Type: text/x-diff
Content-Disposition: inline;
 filename=0001-Schema-qualify-unnest-call-in-ALTER-DATABASE-.-RESET.patch

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


--=-=-=
Content-Type: text/x-diff
Content-Disposition: inline;
 filename=0002-Fix-tab-completion-for-ALTER-ROLE-USER-.-RESET.patch

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


--=-=-=--