Re: Support tab completion for upper character inputs in psql - Mailing list 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