On 7/22/25 13:18, Dagfinn Ilmari Mannsåker wrote: > jian he <jian.universal...@gmail.com> writes: > >> On Thu, Jul 17, 2025 at 1:41 AM Dagfinn Ilmari Mannsåker >> <ilm...@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....@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
From fa26b62298d7a4221d9bcefe71767d337842cabc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilm...@ilmari.org> Date: Mon, 9 Jun 2025 20: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 <ilm...@ilmari.org> Reviewed-by: jian he <jian.universal...@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
From f97c82851b7a8f74d0b3ab633326283a93be0823 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilm...@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 <ilm...@ilmari.org> Reviewed-by: jian he <jian.universal...@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
