From: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
To: Tomas Vondra <tomas@vondra.me>
Cc: jian he <jian.universality@gmail.com>
Cc: Robins Tharakan <tharakan@gmail.com>
Cc: 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
Date: Wed, 30 Jul 2025 11:17:55 +0100
Message-ID: <875xfahtik.fsf@wibble.ilmari.org> (raw)
In-Reply-To: <54b435ab-ce2a-4a65-b4a9-35633dea2b28@vondra.me>
References: <87qzyghw2x.fsf@wibble.ilmari.org>
<CACJufxF5_23CykxYOFYeXMR6_XC+fjjUmY5NRf=JQDWR26q4XA@mail.gmail.com>
<87ecu8iid7.fsf@wibble.ilmari.org>
<54b435ab-ce2a-4a65-b4a9-35633dea2b28@vondra.me>
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
>>>> (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.
>>
>
> 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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: ilmari@ilmari.org, tomas@vondra.me, jian.universality@gmail.com, tharakan@gmail.com, tomas.vondra@postgresql.org, pgsql-hackers@lists.postgresql.org
Subject: Re: Fix tab completion in v18 for ALTER DATABASE/USER/ROLE ... RESET
In-Reply-To: <875xfahtik.fsf@wibble.ilmari.org>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox