Re: Support tab completion for upper character inputs in psql

At Thu, 22 Apr 2021 12:43:42 +0000, "tanghy(dot)fnst(at)fujitsu(dot)com" <tanghy(dot)fnst(at)fujitsu(dot)com> wrote in
> On Wednesday, April 21, 2021 1:24 PM, Peter Smith <smithpb2250(at)gmail(dot)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