Re: Support tab completion for upper character inputs in psql

Kyotaro Horiguchi <horikyota.ntt@gmail.com>

From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
To: tanghy.fnst@fujitsu.com
Cc: smithpb2250@gmail.com, peter.eisentraut@enterprisedb.com, david.zhang@highgo.ca, tgl@sss.pgh.pa.us, pgsql-hackers@lists.postgresql.org
Date: 2021-04-23T02:58:12Z
Lists: pgsql-hackers
At Thu, 22 Apr 2021 12:43:42 +0000, "tanghy.fnst@fujitsu.com" <tanghy.fnst@fujitsu.com> wrote in 
> On Wednesday, April 21, 2021 1:24 PM, Peter Smith <smithpb2250@gmail.com> Wrot> >4. Memory not freed in multiple places?
> oops. Memory free added.

All usages of pg_string_tolower don't need a copy.
So don't we change the function to in-place converter?

> >6. byte_length == 0?
> >The byte_length was not being checked before, so why is the check needed now?
> 
> We need to make sure the empty input to be case sensitive as before(HEAD).
> For example
> 	CREATE TABLE onetab1 (f1 int);
> 	update onetab1 SET [tab]
> 
> Without the check of "byte_length == 0", pg_strdup_keyword_case will make the column name "f1" to be upper case "F1".
> Namely, the output will be " update onetab1 SET F1" which is not so good.
> 
> I added some tab tests for this empty input case, too. 
> 
> >7. test typo "ralation"
> >8. test typo "case-insensitiveq"
> Thanks, typo fixed. 
> 
> Any further comment is very welcome.

 		if (completion_info_charp)
+		{
 			e_info_charp = escape_string(completion_info_charp);
+			if(e_info_charp[0] == '"')
+				completion_case_sensitive = true;
+			else
+			{
+				le_str = pg_string_tolower(e_info_charp);

It seems right to lower completion_info_charp and ..2 but it is not
right that change completion_case_sensitive here, which only affects
the returned candidates.  This change prevents the following operation
from getting the expected completion candidates.

=# create table "T" (a int) partition by range(a);
=# create table c1 partition of "T" for values from (0) to (10);
=# alter table "T" drop partition C<tab>

Is there any reason for doing that?



+				if (byte_length == 0 || completion_case_sensitive)

Is the condition "byte_length == 0 ||" right?

This results in a maybe-unexpected behavior,

=# \set COM_KEYWORD_CASE upper
=# create table t (a int) partition by range(a);
=# create table d1 partition of t for values from (0) to (10);
=# alter table t drop partition <tab>

This results in 

=# alter table t drop partition d1

I think we are expecting D1 as the result.

By the way COMP_KEYWORD_CASE suggests that *keywords* are completed
following the setting. However, they are not keywords, but
identifiers. And some people (including me) might dislike that
keywords and identifiers follow the same setting.  Specifically I
sometimes want keywords to be upper-cased but identifiers (always) be
lower-cased.


regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center



Commits

  1. Fix minor memory leaks in psql's tab completion.

  2. Further tweaks for psql's new tab-completion logic.

  3. Treat case of tab-completion keywords a bit more carefully.

  4. psql: improve tab-complete's handling of variant SQL names.

  5. Make PQcancel use the PGconn's tcp_user_timeout and keepalives settings.

  6. Use a WaitLatch for vacuum/autovacuum sleeping