RE: Support tab completion for upper character inputs in psql

From: "tanghy(dot)fnst(at)fujitsu(dot)com" <tanghy(dot)fnst(at)fujitsu(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>
Cc: Japin Li <japinli(at)hotmail(dot)com>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>, "smithpb2250(at)gmail(dot)com" <smithpb2250(at)gmail(dot)com>, "david(dot)zhang(at)highgo(dot)ca" <david(dot)zhang(at)highgo(dot)ca>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: RE: Support tab completion for upper character inputs in psql
Date: 2022-01-20 07:37:18
Message-ID: OS0PR01MB6113C130461FB8A9DBA0DAEBFB5A9@OS0PR01MB6113.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sunday, January 16, 2022 3:51 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> said:
> Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com> writes:
> > The rest of the patch seems ok in principle, since AFAICT enums are the
> > only query result in tab-complete.c that are not identifiers and thus
> > subject to case issues.
>
> I spent some time looking at this patch. I'm not very happy with it,
> for two reasons:
>
> 1. The downcasing logic in the patch bears very little resemblance
> to the backend's actual downcasing logic, which can be found in
> src/backend/parser/scansup.c's downcase_identifier(). Notably,
> the patch's restriction to only convert all-ASCII strings seems
> indefensible, because that's not how things really work. I fear
> we can't always exactly duplicate the backend's behavior, because
> it's dependent on the server's locale and encoding; but I think
> we should at least get it right in the common case where psql is
> using the same locale and encoding as the server.

Thanks for your suggestion, I removed ASCII strings check function
and added single byte encoding check just like downcase_identifier.
Also added PGCLIENTENCODING setting in the test script to make
test cases pass.
Now the patch supports tab-completion with none-quoted upper characters
available when client encoding is in single byte.

> 2. I don't think there's been much thought about the larger picture
> of what is to be accomplished. Right now, we successfully
> tab-complete inputs that are prefixes of the canonical spelling (per
> quote_identifier) of the object's name, and don't try at all for
> non-canonical spellings. I'm on board with trying to allow some of
> the latter but I'm not sure that this patch represents much forward
> progress. To be definite about it, suppose we have a DB containing
> just two tables whose names start with "m", say mytab and mixedTab.
> Then:
>
> (a) m<TAB> immediately completes mytab, ignoring mixedTab
>
> (b) "m<TAB> immediately completes "mixedTab", ignoring mytab
>
> (c) "my<TAB> fails to find anything
>
> (d) mi<TAB> fails to find anything
>
> (e) M<TAB> fails to find anything
>
> This patch proposes to improve case (e), but to my taste cases (a)
> through (c) are much bigger problems. It'd be nice if (d) worked too
> --- that'd require injecting a double-quote where the user had not
> typed one, but we already do the equivalent thing with single-quotes
> for file names, so why not? (Although after fighting with readline
> yesterday to try to get it to handle single-quoted enum labels sanely,
> I'm not 100% sure if (d) is possible.)
>
> Also, even for case (e), what we have with this patch is that it
> immediately completes mytab, ignoring mixedTab. Is that what we want?
> Another example is that miX<TAB> fails to find anything, which seems
> like a POLA violation given that mY<TAB> completes to mytab.
>
> I'm not certain how many of these alternatives can be supported
> without introducing ambiguity that wasn't there before (which'd
> manifest as failing to complete in cases where the existing code
> chooses an alternative just fine). But I really don't like the
> existing behavior for (b) and (c) --- I should be able to spell
> a name with double quotes if I want, without losing completion
> support.

You are right, it's more convenient in that way.
I haven't thought about it before. By now, the patch suppose:
If user needs to type a table with name in upper character,
they should input the double quotes by themselves. If the double
quote is input by a user, only table name with upper character could be searched.

I may try to implement as you expected but it seems not so easy.
(as you said, without introducing ambiguity that wasn't there before)
I'd appreciate if someone could give me a hint/hand on this.

> BTW, another thing that maybe we should think about is how this
> interacts with the pattern matching capability in \d and friends.
> If people can tab-complete non-canonical spellings, they might
> expect the same spellings to work in \d. I don't say that this
> patch has to fix that, but we might want to look and be sure we're
> not painting ourselves into a corner (especially since I see
> that we already perform tab-completion in that context).

Yes. Agreed, if we solve the previous problem,
meta-command tab completion should also be considered.

Regards,
Tang

Attachment Content-Type Size
v12-0001-Support-tab-completion-with-a-query-result-for-u.patch application/octet-stream 7.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kyotaro Horiguchi 2022-01-20 08:19:04 Re: standby recovery fails (tablespace related) (tentative patch and discussion)
Previous Message Ronan Dunklau 2022-01-20 07:36:46 Re: Use generation context to speed up tuplesorts