Horton

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix tab completion for ALTER ROLE|USER ... RESET

  2. Schema-qualify unnest() in ALTER DATABASE ... RESET

  1. Fix tab completion in v18 for ALTER DATABASE/USER/ROLE ... RESET

    Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> — 2025-07-16T17:41:42Z

    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.

- ilmari

  2. Re: Fix tab completion in v18 for ALTER DATABASE/USER/ROLE ... RESET

    jian he <jian.universality@gmail.com> — 2025-07-22T10:28:18Z

    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.

  3. Re: Fix tab completion in v18 for ALTER DATABASE/USER/ROLE ... RESET

    Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> — 2025-07-22T11:18:28Z

    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

  4. Re: Fix tab completion in v18 for ALTER DATABASE/USER/ROLE ... RESET

    Tomas Vondra <tomas@vondra.me> — 2025-07-29T17:13:13Z

    
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.

I plan to push the attached fixes soon ...


regards

-- 
Tomas Vondra

  5. Re: Fix tab completion in v18 for ALTER DATABASE/USER/ROLE ... RESET

    Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> — 2025-07-30T10:17:55Z

    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

  6. Re: Fix tab completion in v18 for ALTER DATABASE/USER/ROLE ... RESET

    Tomas Vondra <tomas@vondra.me> — 2025-07-31T14:09:05Z

    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

  7. Re: Fix tab completion in v18 for ALTER DATABASE/USER/ROLE ... RESET

    Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> — 2025-07-31T14:21:33Z

    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!

> Thanks for the fixes!
>
> regards