On 10/12/23 10:27, Dagfinn Ilmari Mannsåker wrote:
> Michael Paquier <michael@paquier.xyz> writes:
>
>> On Fri, Apr 14, 2023 at 12:05:25PM +0200, Jim Jones wrote:
>>> The patch applies cleanly and it does what it is proposing. - and it's IMHO
>>> a very nice addition.
>>>
>>> I've marked the CF entry as "Ready for Committer".
>>
>> +/* ... AT TIME ZONE ... */
>> + else if (TailMatches("AT"))
>> + COMPLETE_WITH("TIME ZONE");
>> + else if (TailMatches("AT", "TIME"))
>> + COMPLETE_WITH("ZONE");
>> + else if (TailMatches("AT", "TIME", "ZONE"))
>> + COMPLETE_WITH_TIMEZONE_NAME();
>>
>> This style will for the completion of timezone values even if "AT" is
>> the first word of a query. Shouldn't this be more selective by making
>> sure that we are at least in the context of a SELECT query?
>
> It's valid anywhere an expression is, which is a lot more places than
> just SELECT queries. Off the top of my head I can think of WITH,
> INSERT, UPDATE, VALUES, CALL, CREATE TABLE, CREATE INDEX.
>
> As I mentioned upthread, the only place in the grammar where the word AT
> occurs is in AT TIME ZONE, so there's no ambiguity. Also, it doesn't
> complete time zone names after AT, it completes the literal words TIME
> ZONE, and you have to then hit tab again to get a list of time zones.
> If we (or the SQL committee) were to invent more operators that start
> with the word AT, we can add those to the first if clause above and
> complete with the appropriate values after each one separately.
Speaking of this...
The SQL committee already has another operator starting with AT which is
AT LOCAL. I am implementing it in
https://commitfest.postgresql.org/45/4343/ where I humbly admit that I
did not think of psql tab completion at all.
These two patches are co-dependent and whichever goes in first the other
will need to be adjusted accordingly.
--
Vik Fearing