Support tab completion for upper character inputs in psql

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

Support tab completion for upper character inputs in psql

Tang, Haiying
Hi Hackers,

When using psql I found there's no tab completion for upper character inputs. It's really inconvenient sometimes so I try to fix this problem in the attached patch.

Here is the examples to show what this patch can do.
Action:
1. connect the db using psql
2. input SQL command
3. enter TAB key(twice at the very first time)

Results:
[master]
postgres=# set a
all                      allow_system_table_mods  application_name         array_nulls
postgres=# set A

postgres=# set A

[patched]
postgres=# set a
all                      allow_system_table_mods  application_name         array_nulls
postgres=# set A
ALL                      ALLOW_SYSTEM_TABLE_MODS  APPLICATION_NAME         ARRAY_NULLS
postgres=# set A

Please take a check at this patch. Any comment is welcome.

Regards,
Tang



0001-Support-tab-completion-for-upper-character-inputs-in.patch (5K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Support tab completion for upper character inputs in psql

Tom Lane-2
"Tang, Haiying" <[hidden email]> writes:
> When using psql I found there's no tab completion for upper character inputs. It's really inconvenient sometimes so I try to fix this problem in the attached patch.

This looks like you're trying to force case-insensitive behavior
whether that is appropriate or not.  Does not sound like a good
idea.

                        regards, tom lane


Reply | Threaded
Open this post in threaded view
|

Re: Support tab completion for upper character inputs in psql

Kyotaro Horiguchi-4
At Sun, 07 Feb 2021 13:55:00 -0500, Tom Lane <[hidden email]> wrote in
> "Tang, Haiying" <[hidden email]> writes:
> > When using psql I found there's no tab completion for upper character inputs. It's really inconvenient sometimes so I try to fix this problem in the attached patch.
>
> This looks like you're trying to force case-insensitive behavior
> whether that is appropriate or not.  Does not sound like a good
> idea.

Agreed. However I'm not sure what the OP exactly wants, \set behaves
in a different but similar way.

=# \set c[tab]
=# \set COMP_KEYWORD_CASE _

However set doesn't. If it is what is wanted, the following change on
Query_for_list_of_set_vars works (only for the case of SET/RESET
commands).


diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 5f0e775fd3..5c2a263785 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -725,7 +725,8 @@ static const SchemaQuery Query_for_list_of_statistics = {
 "  UNION ALL SELECT 'role' "\
 "  UNION ALL SELECT 'tablespace' "\
 "  UNION ALL SELECT 'all') ss "\
-" WHERE substring(name,1,%d)='%s'"
+" WHERE substring(name,1,%1$d)='%2$s' "\
+"    OR pg_catalog.lower(substring(name,1,%1$d))=pg_catalog.lower('%2$s')"
 
 #define Query_for_list_of_show_vars \
 "SELECT name FROM "\

=# set AP[tab]
=# set application_name _

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center


Reply | Threaded
Open this post in threaded view
|

RE: Support tab completion for upper character inputs in psql

Tang, Haiying
At Sun, 07 Feb 2021 13:55:00 -0500, Tom Lane <[hidden email]> wrote in
>
> This looks like you're trying to force case-insensitive behavior
> whether that is appropriate or not.  Does not sound like a good idea.

Thanks for your reply.
I raise this issue because I thought all SQL command should be case-insensitive.
And the set/reset/show commands work well no matter the input configuration parameter is in upper or in lower case.
My modification is not good enough, but I really think it's more convenient if we can support the tab-completion for upper character inputs.

=# set APPLICATION_NAME to test;
SET

=# show APPLICATION_name;
 application_name
------------------
 test
(1 row)

From: Kyotaro Horiguchi <[hidden email]>
Sent: Monday, February 8, 2021 5:02 PM

>However set doesn't. If it is what is wanted, the following change on Query_for_list_of_set_vars works (only for the case of SET/RESET commands).

Thanks for your update. I applied your patch, it works well for SET/RESET commands.
I added the same modification to SHOW command. The new patch(V2) can support tab completion for upper character inputs in psql for SET/RESET/SHOW commands.

Regards,
Tang



V2-0001-Support-tab-completion-for-upper-character-inputs-in.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

RE: Support tab completion for upper character inputs in psql

Tang, Haiying
At Sun, 07 Feb 2021 13:55:00 -0500, Tom Lane <[hidden email]> wrote in
>
> This looks like you're trying to force case-insensitive behavior
> whether that is appropriate or not.  Does not sound like a good idea.

I'm still confused about the APPROPRIATE behavior of tab completion.
It seems ALTER table/tablespace <name> SET/RESET is already case-insensitive.

For example
# alter tablespace dbspace set(e[tab]
# alter tablespace dbspace set(effective_io_concurrency

# alter tablespace dbspace set(E[tab]
# alter tablespace dbspace set(EFFECTIVE_IO_CONCURRENCY

The above behavior is exactly the same as what the patch(attached in the following message) did for SET/RESET etc.
https://www.postgresql.org/message-id/flat/a63cbd45e3884cf9b3961c2a6a95dcb7%40G08CNEXMBPEKD05.g08.fujitsu.local

If anyone can share me some cases which show inappropriate scenarios of forcing case-insensitive inputs in psql.
I'd be grateful for that.

Regards,
Tang





Reply | Threaded
Open this post in threaded view
|

Re: Support tab completion for upper character inputs in psql

Peter Eisentraut-7
On 09.02.21 15:48, Tang, Haiying wrote:
> I'm still confused about the APPROPRIATE behavior of tab completion.
> It seems ALTER table/tablespace <name> SET/RESET is already case-insensitive.
>
> For example
> # alter tablespace dbspace set(e[tab]
> # alter tablespace dbspace set(effective_io_concurrency
>
> # alter tablespace dbspace set(E[tab]
> # alter tablespace dbspace set(EFFECTIVE_IO_CONCURRENCY

This case completes with a hardcoded list, which is done  
case-insensitively by default.  The cases that complete with a query  
result are not case insensitive right now.  This affects things like

UPDATE T<tab>

as well.  I think your first patch was basically right.  But we need to  
understand that this affects all completions with query results, not  
just the one you wanted to fix.  So you should analyze all the callers  
and explain why the proposed change is appropriate.


Reply | Threaded
Open this post in threaded view
|

RE: Support tab completion for upper character inputs in psql

tanghy.fnst@fujitsu.com
On Tuesday, March 16, 2021 5:20 AM, Peter Eisentraut <[hidden email]> wrote:

>The cases that complete with a query  
>result are not case insensitive right now.  This affects things like
>
>UPDATE T<tab>
>
>as well.  I think your first patch was basically right.  But we need to  
>understand that this affects all completions with query results, not  
>just the one you wanted to fix.  So you should analyze all the callers  
>and explain why the proposed change is appropriate.

Thanks for your review and suggestion. Please find attached patch V3 which was based on the first patch[1].
Difference from the first patch is:

Add tab completion support for all query results in psql.
complete_from_query
+complete_from_versioned_query
+complete_from_schema_query
+complete_from_versioned_schema_query

[1] https://www.postgresql.org/message-id/a63cbd45e3884cf9b3961c2a6a95dcb7%40G08CNEXMBPEKD05.g08.fujitsu.local

The modification to support case insensitive matching in " _complete_from_query" is based on "complete_from_const and "complete_from_list" .
Please let me know if you find anything insufficient.

Regards,
Tang



V3-0001-Support-tab-completion-with-a-query-result-for-upper.patch (6K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Support tab completion for upper character inputs in psql

David Zhang
Hi Tang,

Thanks a lot for the patch.

I did a quick test based on the latest patch V3 on latest master branch
"commit 4753ef37e0eda4ba0af614022d18fcbc5a946cc9".

Case 1: before patch

   1 postgres=# set a
   2 all                      allow_system_table_mods
application_name         array_nulls
   3 postgres=# set A
   4
   5 postgres=# create TABLE tbl (data text);
   6 CREATE TABLE
   7 postgres=# update tbl SET DATA =
   8
   9 postgres=# update T
  10
  11 postgres=#

Case 2: after patched

   1 postgres=# set a
   2 all                      allow_system_table_mods
application_name         array_nulls
   3 postgres=# set A
   4 ALL                      ALLOW_SYSTEM_TABLE_MODS
APPLICATION_NAME         ARRAY_NULLS
   5 postgres=# create TABLE tbl (data text);
   6 CREATE TABLE
   7
   8 postgres=# update tbl SET DATA =
   9
  10 postgres=# update TBL SET
  11
  12 postgres=#

So, as you can see the difference is between line 8 and 10 in case 2. It
looks like the lowercase can auto complete more than the uppercase;
secondly, if you can add some test cases, it would be great.

Best regards,
David

On 2021-03-22 5:41 a.m., [hidden email] wrote:

> On Tuesday, March 16, 2021 5:20 AM, Peter Eisentraut <[hidden email]> wrote:
>
>> The cases that complete with a query
>> result are not case insensitive right now.  This affects things like
>>
>> UPDATE T<tab>
>>
>> as well.  I think your first patch was basically right.  But we need to
>> understand that this affects all completions with query results, not
>> just the one you wanted to fix.  So you should analyze all the callers
>> and explain why the proposed change is appropriate.
> Thanks for your review and suggestion. Please find attached patch V3 which was based on the first patch[1].
> Difference from the first patch is:
>
> Add tab completion support for all query results in psql.
> complete_from_query
> +complete_from_versioned_query
> +complete_from_schema_query
> +complete_from_versioned_schema_query
>
> [1] https://www.postgresql.org/message-id/a63cbd45e3884cf9b3961c2a6a95dcb7%40G08CNEXMBPEKD05.g08.fujitsu.local
>
> The modification to support case insensitive matching in " _complete_from_query" is based on "complete_from_const and "complete_from_list" .
> Please let me know if you find anything insufficient.
>
> Regards,
> Tang
>
>
--
David

Software Engineer
Highgo Software Inc. (Canada)
www.highgo.ca


Reply | Threaded
Open this post in threaded view
|

RE: Support tab completion for upper character inputs in psql

tanghy.fnst@fujitsu.com
On Wednesday, March 31, 2021 4:05 AM, David Zhang <[hidden email]> wrote

>   8 postgres=# update tbl SET DATA =
>   9
>  10 postgres=# update TBL SET
>  11
>  12 postgres=#
>
>So, as you can see the difference is between line 8 and 10 in case 2. It
>looks like the lowercase can auto complete more than the uppercase;
>secondly, if you can add some test cases, it would be great.

Thanks for your test. I fix the bug and add some tests for it.
Please find attached the latest patch V4.

Differences from v3 are:
* fix an issue reported by Zhang [1] where a scenario was found which still wasn't able to realize tap completion in query.
* add some tap tests.

[1] https://www.postgresql.org/message-id/3140db2a-9808-c470-7e60-de39c431b3ab%40highgo.ca

Regards,
Tang

V4-0001-Support-tab-completion-with-a-query-result-for-upper.patch (7K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Support tab completion for upper character inputs in psql

Peter Eisentraut-7
On 01.04.21 11:40, [hidden email] wrote:

> On Wednesday, March 31, 2021 4:05 AM, David Zhang <[hidden email]> wrote
>
>>    8 postgres=# update tbl SET DATA =
>>    9
>>   10 postgres=# update TBL SET
>>   11
>>   12 postgres=#
>>
>> So, as you can see the difference is between line 8 and 10 in case 2. It
>> looks like the lowercase can auto complete more than the uppercase;
>> secondly, if you can add some test cases, it would be great.
>
> Thanks for your test. I fix the bug and add some tests for it.
> Please find attached the latest patch V4.
>
> Differences from v3 are:
> * fix an issue reported by Zhang [1] where a scenario was found which still wasn't able to realize tap completion in query.
> * add some tap tests.

Seeing the tests you provided, it's pretty obvious that the current
behavior is insufficient.  I think we could probably think of a few more
tests, for example exercising the "If case insensitive matching was
requested initially, adjust the case according to setting." case, or
something with quoted identifiers.  I'll push this to the next commit
fest for now.  I encourage you to keep working on it.