pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feed[PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion
22+ messages / 5 participants
[nested] [flat]
Commits from this thread:
2021-01-14 3f238b882c Improve tab-completion for CLOSE, DECLARE, FETCH and MOVE.
* [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion
@ 2020-12-09 03:57 Shinya11.Kato@nttdata.com
2021-01-04 11:56 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
0 siblings, 1 reply; 22+ messages in thread
From: Shinya11.Kato@nttdata.com @ 2020-12-09 03:57 UTC (permalink / raw)
To: pgsql-hackers
Hi!
I created a patch for improving CLOSE, FETCH, MOVE tab completion.
Specifically, I add CLOSE, FETCH, MOVE tab completion for completing a predefined cursors.
Regards,
Shinya Kato
Attachments:
fix_tab_complete_close_fetch_move.patch (application/octet-stream, 3.6K) download
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 3a43c09bf6..6b5b326c58 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -976,6 +976,9 @@ static const SchemaQuery Query_for_list_of_statistics = {
" and pg_catalog.pg_table_is_visible(c2.oid)"\
" and c2.relispartition = 'true'"
+#define Query_for_list_of_cursors \
+" SELECT name FROM pg_cursors"\
+
/*
* These object types were introduced later than our support cutoff of
* server version 7.4. We use the VersionedQuery infrastructure so that
@@ -2284,6 +2287,10 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH_VERSIONED_SCHEMA_QUERY(Query_for_list_of_procedures, NULL);
else if (Matches("CALL", MatchAny))
COMPLETE_WITH("(");
+/* CLOSE */
+ else if (Matches("CLOSE"))
+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors
+ " UNION ALL SELECT 'ALL'");
/* CLUSTER */
else if (Matches("CLUSTER"))
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_clusterables, "UNION SELECT 'VERBOSE'");
@@ -3166,26 +3173,44 @@ psql_completion(const char *text, int start, int end)
/* FETCH && MOVE */
/*
- * Complete FETCH with one of ABSOLUTE, BACKWARD, FORWARD, RELATIVE, ALL,
- * NEXT, PRIOR, FIRST, LAST
+ * Complete FETCH with a list of cursors and one of ABSOLUTE, BACKWARD, FORWARD, RELATIVE, ALL,
+ * NEXT, PRIOR, FIRST, LAST, FROM, IN
*/
else if (Matches("FETCH|MOVE"))
- COMPLETE_WITH("ABSOLUTE", "BACKWARD", "FORWARD", "RELATIVE",
- "ALL", "NEXT", "PRIOR", "FIRST", "LAST");
-
- /* Complete FETCH BACKWARD or FORWARD with one of ALL, FROM, IN */
+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors
+ " UNION SELECT 'ABSOLUTE'"
+ " UNION SELECT 'BACKWARD'"
+ " UNION SELECT 'FORWARD'"
+ " UNION SELECT 'RELATIVE'"
+ " UNION SELECT 'ALL'"
+ " UNION SELECT 'NEXT'"
+ " UNION SELECT 'PRIOR'"
+ " UNION SELECT 'FIRST'"
+ " UNION SELECT 'LAST'"
+ " UNION SELECT 'FROM'"
+ " UNION SELECT 'IN'"
+ );
+
+ /* Complete FETCH BACKWARD or FORWARD with a list of cursors and one of ALL, FROM, IN */
else if (Matches("FETCH|MOVE", "BACKWARD|FORWARD"))
- COMPLETE_WITH("ALL", "FROM", "IN");
+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors
+ " UNION SELECT 'ALL'"
+ " UNION SELECT 'FROM'"
+ " UNION SELECT 'IN'"
+ );
- /*
- * Complete FETCH <direction> with "FROM" or "IN". These are equivalent,
- * but we may as well tab-complete both: perhaps some users prefer one
- * variant or the other.
- */
+ /* Complete FETCH <direction> with a list of cursors and "FROM" or "IN" */
else if (Matches("FETCH|MOVE", "ABSOLUTE|BACKWARD|FORWARD|RELATIVE",
MatchAnyExcept("FROM|IN")) ||
Matches("FETCH|MOVE", "ALL|NEXT|PRIOR|FIRST|LAST"))
- COMPLETE_WITH("FROM", "IN");
+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors
+ " UNION SELECT 'FROM'"
+ " UNION SELECT 'IN'"
+ );
+ /* Complete FETCH <direction> FROM or IN with a list of cursors */
+ else if (HeadMatches("FETCH|MOVE") &&
+ TailMatches("FROM|IN"))
+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors);
/* FOREIGN DATA WRAPPER */
/* applies in ALTER/DROP FDW and in CREATE SERVER */
* Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion
2020-12-09 03:57 [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
@ 2021-01-04 11:56 ` Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-05 06:02 ` RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
0 siblings, 1 reply; 22+ messages in thread
From: Masahiko Sawada @ 2021-01-04 11:56 UTC (permalink / raw)
To: Shinya11.Kato@nttdata.com; +Cc: pgsql-hackers
On Wed, Dec 9, 2020 at 12:59 PM <Shinya11.Kato@nttdata.com> wrote:
>
> Hi!
>
>
>
> I created a patch for improving CLOSE, FETCH, MOVE tab completion.
>
> Specifically, I add CLOSE, FETCH, MOVE tab completion for completing a predefined cursors.
>
Thank you for the patch!
When I applied the patch, I got the following whitespace warnings:
$ git apply ~/patches/fix_tab_complete_close_fetch_move.patch
/home/masahiko/patches/fix_tab_complete_close_fetch_move.patch:40:
indent with spaces.
COMPLETE_WITH_QUERY(Query_for_list_of_cursors
/home/masahiko/patches/fix_tab_complete_close_fetch_move.patch:41:
indent with spaces.
" UNION SELECT 'ABSOLUTE'"
/home/masahiko/patches/fix_tab_complete_close_fetch_move.patch:42:
indent with spaces.
" UNION SELECT 'BACKWARD'"
/home/masahiko/patches/fix_tab_complete_close_fetch_move.patch:43:
indent with spaces.
" UNION SELECT 'FORWARD'"
/home/masahiko/patches/fix_tab_complete_close_fetch_move.patch:44:
indent with spaces.
" UNION SELECT 'RELATIVE'"
warning: squelched 19 whitespace errors
warning: 24 lines add whitespace errors.
I recommend you checking whitespaces or running pgindent.
Here are some comments:
/*
- * Complete FETCH with one of ABSOLUTE, BACKWARD, FORWARD, RELATIVE, ALL,
- * NEXT, PRIOR, FIRST, LAST
+ * Complete FETCH with a list of cursors and one of ABSOLUTE,
BACKWARD, FORWARD, RELATIVE, ALL,
+ * NEXT, PRIOR, FIRST, LAST, FROM, IN
*/
Maybe I think the commend should say:
+ * Complete FETCH with one of ABSOLUTE, BACKWARD, FORWARD, RELATIVE, ALL,
+ * NEXT, PRIOR, FIRST, LAST, FROM, IN, and a list of cursors
Other updates of the comment seem to have the same issue.
Or I think we can omit the details from the comment since it seems
redundant information. We can read it from the arguments of the
following COMPLETE_WITH_QUERY().
---
- /*
- * Complete FETCH <direction> with "FROM" or "IN". These are equivalent,
- * but we may as well tab-complete both: perhaps some users prefer one
- * variant or the other.
- */
+ /* Complete FETCH <direction> with a list of cursors and "FROM" or "IN" */
Why did you remove the second sentence in the above comment?
---
The patch improves tab completion for CLOSE, FETCH, and MOVE but is
there any reason why you didn't do that for DECLARE? I think DECLARE
also can be improved and it's a good timing for that.
Regards,
--
Masahiko Sawada
EnterpriseDB: https://www.enterprisedb.com/
^ permalink raw reply [nested|flat] 22+ messages in thread
* RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion
2020-12-09 03:57 [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-04 11:56 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
@ 2021-01-05 06:02 ` Shinya11.Kato@nttdata.com
2021-01-05 09:08 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@oss.nttdata.com>
2021-01-05 09:56 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
0 siblings, 2 replies; 22+ messages in thread
From: Shinya11.Kato@nttdata.com @ 2021-01-05 06:02 UTC (permalink / raw)
To: sawada.mshk@gmail.com; +Cc: pgsql-hackers
Thank you for your review!
I fixed some codes and attach a new patch.
>When I applied the patch, I got the following whitespace warnings:
>
>$ git apply ~/patches/fix_tab_complete_close_fetch_move.patch
>/home/masahiko/patches/fix_tab_complete_close_fetch_move.patch:40:
>indent with spaces.
> COMPLETE_WITH_QUERY(Query_for_list_of_cursors
>/home/masahiko/patches/fix_tab_complete_close_fetch_move.patch:41:
>indent with spaces.
> " UNION SELECT 'ABSOLUTE'"
>/home/masahiko/patches/fix_tab_complete_close_fetch_move.patch:42:
>indent with spaces.
> " UNION SELECT 'BACKWARD'"
>/home/masahiko/patches/fix_tab_complete_close_fetch_move.patch:43:
>indent with spaces.
> " UNION SELECT 'FORWARD'"
>/home/masahiko/patches/fix_tab_complete_close_fetch_move.patch:44:
>indent with spaces.
> " UNION SELECT 'RELATIVE'"
>warning: squelched 19 whitespace errors
>warning: 24 lines add whitespace errors.
>
>I recommend you checking whitespaces or running pgindent.
Thank you for your advice and I have corrected it.
>Here are some comments:
>
> /*
>- * Complete FETCH with one of ABSOLUTE, BACKWARD, FORWARD,
>RELATIVE, ALL,
>- * NEXT, PRIOR, FIRST, LAST
>+ * Complete FETCH with a list of cursors and one of ABSOLUTE,
>BACKWARD, FORWARD, RELATIVE, ALL,
>+ * NEXT, PRIOR, FIRST, LAST, FROM, IN
> */
>
>Maybe I think the commend should say:
>
>+ * Complete FETCH with one of ABSOLUTE, BACKWARD, FORWARD,
>RELATIVE, ALL,
>+ * NEXT, PRIOR, FIRST, LAST, FROM, IN, and a list of cursors
>
>Other updates of the comment seem to have the same issue.
>
>Or I think we can omit the details from the comment since it seems redundant
>information. We can read it from the arguments of the following
>COMPLETE_WITH_QUERY().
It certainly seems redundant, so I deleted them.
>---
>- /*
>- * Complete FETCH <direction> with "FROM" or "IN". These are equivalent,
>- * but we may as well tab-complete both: perhaps some users prefer one
>- * variant or the other.
>- */
>+ /* Complete FETCH <direction> with a list of cursors and "FROM" or
>+ "IN" */
>
>Why did you remove the second sentence in the above comment?
I had misunderstood the meaning and deleted it.
I deleted it as well as above, but would you prefer it to be there?
>---
>The patch improves tab completion for CLOSE, FETCH, and MOVE but is there
>any reason why you didn't do that for DECLARE? I think DECLARE also can be
>improved and it's a good timing for that.
I wanted to improve tab completion for DECLARE, but I couldn't find anything to improve.
Please let me know if there are any codes that can be improved.
Regards,
Shinya Kato
Attachments:
fix_tab_complete_close_fetch_move_v2.patch (application/octet-stream, 2.8K) download
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 3a43c09bf6..96f3fe8b9f 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -976,6 +976,9 @@ static const SchemaQuery Query_for_list_of_statistics = {
" and pg_catalog.pg_table_is_visible(c2.oid)"\
" and c2.relispartition = 'true'"
+#define Query_for_list_of_cursors \
+" SELECT name FROM pg_cursors"\
+
/*
* These object types were introduced later than our support cutoff of
* server version 7.4. We use the VersionedQuery infrastructure so that
@@ -2284,6 +2287,10 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH_VERSIONED_SCHEMA_QUERY(Query_for_list_of_procedures, NULL);
else if (Matches("CALL", MatchAny))
COMPLETE_WITH("(");
+/* CLOSE */
+ else if (Matches("CLOSE"))
+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors
+ " UNION ALL SELECT 'ALL'");
/* CLUSTER */
else if (Matches("CLUSTER"))
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_clusterables, "UNION SELECT 'VERBOSE'");
@@ -3164,28 +3171,33 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH("SELECT", "INSERT", "DELETE", "UPDATE", "DECLARE");
/* FETCH && MOVE */
-
- /*
- * Complete FETCH with one of ABSOLUTE, BACKWARD, FORWARD, RELATIVE, ALL,
- * NEXT, PRIOR, FIRST, LAST
- */
else if (Matches("FETCH|MOVE"))
- COMPLETE_WITH("ABSOLUTE", "BACKWARD", "FORWARD", "RELATIVE",
- "ALL", "NEXT", "PRIOR", "FIRST", "LAST");
-
- /* Complete FETCH BACKWARD or FORWARD with one of ALL, FROM, IN */
+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors
+ " UNION SELECT 'ABSOLUTE'"
+ " UNION SELECT 'BACKWARD'"
+ " UNION SELECT 'FORWARD'"
+ " UNION SELECT 'RELATIVE'"
+ " UNION SELECT 'ALL'"
+ " UNION SELECT 'NEXT'"
+ " UNION SELECT 'PRIOR'"
+ " UNION SELECT 'FIRST'"
+ " UNION SELECT 'LAST'"
+ " UNION SELECT 'FROM'"
+ " UNION SELECT 'IN'");
else if (Matches("FETCH|MOVE", "BACKWARD|FORWARD"))
- COMPLETE_WITH("ALL", "FROM", "IN");
-
- /*
- * Complete FETCH <direction> with "FROM" or "IN". These are equivalent,
- * but we may as well tab-complete both: perhaps some users prefer one
- * variant or the other.
- */
+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors
+ " UNION SELECT 'ALL'"
+ " UNION SELECT 'FROM'"
+ " UNION SELECT 'IN'");
else if (Matches("FETCH|MOVE", "ABSOLUTE|BACKWARD|FORWARD|RELATIVE",
MatchAnyExcept("FROM|IN")) ||
Matches("FETCH|MOVE", "ALL|NEXT|PRIOR|FIRST|LAST"))
- COMPLETE_WITH("FROM", "IN");
+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors
+ " UNION SELECT 'FROM'"
+ " UNION SELECT 'IN'");
+ else if (HeadMatches("FETCH|MOVE") &&
+ TailMatches("FROM|IN"))
+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors);
/* FOREIGN DATA WRAPPER */
/* applies in ALTER/DROP FDW and in CREATE SERVER */
* Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion
2020-12-09 03:57 [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-04 11:56 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-05 06:02 ` RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
@ 2021-01-05 09:08 ` Fujii Masao <masao.fujii@oss.nttdata.com>
2021-01-06 02:13 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
1 sibling, 1 reply; 22+ messages in thread
From: Fujii Masao @ 2021-01-05 09:08 UTC (permalink / raw)
To: Shinya11.Kato@nttdata.com; sawada.mshk@gmail.com; +Cc: pgsql-hackers
On 2021/01/05 15:02, Shinya11.Kato@nttdata.com wrote:
> Thank you for your review!
> I fixed some codes and attach a new patch.
Thanks for updating the patch!
+#define Query_for_list_of_cursors \
+" SELECT name FROM pg_cursors"\
This query should be the following?
" SELECT pg_catalog.quote_ident(name) "\
" FROM pg_catalog.pg_cursors "\
" WHERE substring(pg_catalog.quote_ident(name),1,%d)='%s'"
+/* CLOSE */
+ else if (Matches("CLOSE"))
+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors
+ " UNION ALL SELECT 'ALL'");
"UNION ALL" should be "UNION"?
+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors
+ " UNION SELECT 'ABSOLUTE'"
+ " UNION SELECT 'BACKWARD'"
+ " UNION SELECT 'FORWARD'"
+ " UNION SELECT 'RELATIVE'"
+ " UNION SELECT 'ALL'"
+ " UNION SELECT 'NEXT'"
+ " UNION SELECT 'PRIOR'"
+ " UNION SELECT 'FIRST'"
+ " UNION SELECT 'LAST'"
+ " UNION SELECT 'FROM'"
+ " UNION SELECT 'IN'");
This change makes psql unexpectedly output "FROM" and "IN" just after "FETCH".
>
>> When I applied the patch, I got the following whitespace warnings:
>>
>> $ git apply ~/patches/fix_tab_complete_close_fetch_move.patch
>> /home/masahiko/patches/fix_tab_complete_close_fetch_move.patch:40:
>> indent with spaces.
>> COMPLETE_WITH_QUERY(Query_for_list_of_cursors
>> /home/masahiko/patches/fix_tab_complete_close_fetch_move.patch:41:
>> indent with spaces.
>> " UNION SELECT 'ABSOLUTE'"
>> /home/masahiko/patches/fix_tab_complete_close_fetch_move.patch:42:
>> indent with spaces.
>> " UNION SELECT 'BACKWARD'"
>> /home/masahiko/patches/fix_tab_complete_close_fetch_move.patch:43:
>> indent with spaces.
>> " UNION SELECT 'FORWARD'"
>> /home/masahiko/patches/fix_tab_complete_close_fetch_move.patch:44:
>> indent with spaces.
>> " UNION SELECT 'RELATIVE'"
>> warning: squelched 19 whitespace errors
>> warning: 24 lines add whitespace errors.
>>
>> I recommend you checking whitespaces or running pgindent.
>
> Thank you for your advice and I have corrected it.
>
>> Here are some comments:
>>
>> /*
>> - * Complete FETCH with one of ABSOLUTE, BACKWARD, FORWARD,
>> RELATIVE, ALL,
>> - * NEXT, PRIOR, FIRST, LAST
>> + * Complete FETCH with a list of cursors and one of ABSOLUTE,
>> BACKWARD, FORWARD, RELATIVE, ALL,
>> + * NEXT, PRIOR, FIRST, LAST, FROM, IN
>> */
>>
>> Maybe I think the commend should say:
>>
>> + * Complete FETCH with one of ABSOLUTE, BACKWARD, FORWARD,
>> RELATIVE, ALL,
>> + * NEXT, PRIOR, FIRST, LAST, FROM, IN, and a list of cursors
>>
>> Other updates of the comment seem to have the same issue.
>>
>> Or I think we can omit the details from the comment since it seems redundant
>> information. We can read it from the arguments of the following
>> COMPLETE_WITH_QUERY().
>
> It certainly seems redundant, so I deleted them.
I think that it's better to update and keep those comments rather than removing them.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion
2020-12-09 03:57 [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-04 11:56 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-05 06:02 ` RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-05 09:08 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@oss.nttdata.com>
@ 2021-01-06 02:13 ` Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-06 02:33 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@oss.nttdata.com>
0 siblings, 1 reply; 22+ messages in thread
From: Masahiko Sawada @ 2021-01-06 02:13 UTC (permalink / raw)
To: Fujii Masao <masao.fujii@oss.nttdata.com>; +Cc: pgsql-hackers
On Tue, Jan 5, 2021 at 6:08 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>
>
> + COMPLETE_WITH_QUERY(Query_for_list_of_cursors
> + " UNION SELECT 'ABSOLUTE'"
> + " UNION SELECT 'BACKWARD'"
> + " UNION SELECT 'FORWARD'"
> + " UNION SELECT 'RELATIVE'"
> + " UNION SELECT 'ALL'"
> + " UNION SELECT 'NEXT'"
> + " UNION SELECT 'PRIOR'"
> + " UNION SELECT 'FIRST'"
> + " UNION SELECT 'LAST'"
> + " UNION SELECT 'FROM'"
> + " UNION SELECT 'IN'");
>
> This change makes psql unexpectedly output "FROM" and "IN" just after "FETCH".
I think "FROM" and "IN" can be placed just after "FETCH". According to
the documentation, the direction can be empty. For instance, we can do
like:
postgres(1:7668)=# begin;
BEGIN
postgres(1:7668)=# declare test cursor for select relname from pg_class;
DECLARE CURSOR
postgres(1:7668)=# fetch from test;
relname
--------------
pg_statistic
(1 row)
postgres(1:7668)=# fetch in test;
relname
---------
pg_type
(1 row)
Regards,
--
Masahiko Sawada
EnterpriseDB: https://www.enterprisedb.com/
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion
2020-12-09 03:57 [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-04 11:56 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-05 06:02 ` RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-05 09:08 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@oss.nttdata.com>
2021-01-06 02:13 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
@ 2021-01-06 02:33 ` Fujii Masao <masao.fujii@oss.nttdata.com>
2021-01-06 06:36 ` RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
0 siblings, 1 reply; 22+ messages in thread
From: Fujii Masao @ 2021-01-06 02:33 UTC (permalink / raw)
To: Masahiko Sawada <sawada.mshk@gmail.com>; +Cc: pgsql-hackers
On 2021/01/06 11:13, Masahiko Sawada wrote:
> On Tue, Jan 5, 2021 at 6:08 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>>
>>
>> + COMPLETE_WITH_QUERY(Query_for_list_of_cursors
>> + " UNION SELECT 'ABSOLUTE'"
>> + " UNION SELECT 'BACKWARD'"
>> + " UNION SELECT 'FORWARD'"
>> + " UNION SELECT 'RELATIVE'"
>> + " UNION SELECT 'ALL'"
>> + " UNION SELECT 'NEXT'"
>> + " UNION SELECT 'PRIOR'"
>> + " UNION SELECT 'FIRST'"
>> + " UNION SELECT 'LAST'"
>> + " UNION SELECT 'FROM'"
>> + " UNION SELECT 'IN'");
>>
>> This change makes psql unexpectedly output "FROM" and "IN" just after "FETCH".
>
> I think "FROM" and "IN" can be placed just after "FETCH". According to
> the documentation, the direction can be empty.
You're right. Thanks for correcting me!
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
^ permalink raw reply [nested|flat] 22+ messages in thread
* RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion
2020-12-09 03:57 [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-04 11:56 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-05 06:02 ` RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-05 09:08 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@oss.nttdata.com>
2021-01-06 02:13 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-06 02:33 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@oss.nttdata.com>
@ 2021-01-06 06:36 ` Shinya11.Kato@nttdata.com
2021-01-07 01:01 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
0 siblings, 1 reply; 22+ messages in thread
From: Shinya11.Kato@nttdata.com @ 2021-01-06 06:36 UTC (permalink / raw)
To: masao.fujii@oss.nttdata.com; sawada.mshk@gmail.com; +Cc: pgsql-hackers
>+#define Query_for_list_of_cursors \
>+" SELECT name FROM pg_cursors"\
>
>This query should be the following?
>
>" SELECT pg_catalog.quote_ident(name) "\
>" FROM pg_catalog.pg_cursors "\
>" WHERE substring(pg_catalog.quote_ident(name),1,%d)='%s'"
>
>+/* CLOSE */
>+ else if (Matches("CLOSE"))
>+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors
>+ " UNION ALL SELECT 'ALL'");
>
>"UNION ALL" should be "UNION"?
Thank you for your advice, and I corrected them.
>> + COMPLETE_WITH_QUERY(Query_for_list_of_cursors
>> + " UNION SELECT 'ABSOLUTE'"
>> + " UNION SELECT 'BACKWARD'"
>> + " UNION SELECT 'FORWARD'"
>> + " UNION SELECT 'RELATIVE'"
>> + " UNION SELECT 'ALL'"
>> + " UNION SELECT 'NEXT'"
>> + " UNION SELECT 'PRIOR'"
>> + " UNION SELECT 'FIRST'"
>> + " UNION SELECT 'LAST'"
>> + " UNION SELECT 'FROM'"
>> + " UNION SELECT 'IN'");
>>
>> This change makes psql unexpectedly output "FROM" and "IN" just after "FETCH".
>
>I think "FROM" and "IN" can be placed just after "FETCH". According to
>the documentation, the direction can be empty. For instance, we can do
>like:
Thank you!
>I've attached the patch improving the tab completion for DECLARE as an
>example. What do you think?
>
>BTW according to the documentation, the options of DECLARE statement
>(BINARY, INSENSITIVE, SCROLL, and NO SCROLL) are order-sensitive.
>
>DECLARE name [ BINARY ] [ INSENSITIVE ] [ [ NO ] SCROLL ]
> CURSOR [ { WITH | WITHOUT } HOLD ] FOR query
>
>But I realized that these options are actually order-insensitive. For
>instance, we can declare a cursor like:
>
>=# declare abc scroll binary cursor for select * from pg_class;
>DECLARE CURSOR
>
>The both parser code and documentation has been unchanged from 2003.
>Is it a documentation bug?
Thank you for your patch, and it is good.
I cannot find the description "(BINARY, INSENSITIVE, SCROLL, and NO SCROLL) are order-sensitive."
I saw "The key words BINARY, INSENSITIVE, and SCROLL can appear in any order." , according to the documentation.
I made a new patch, but the amount of codes was large due to order-insensitive.
If you know of a better way, please let me know.
Regards,
Shinya Kato
Attachments:
fix_tab_complete_close_fetch_move_v3.patch (application/octet-stream, 5.6K) download
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 3a43c09bf6..4ed241a4ee 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -976,6 +976,11 @@ static const SchemaQuery Query_for_list_of_statistics = {
" and pg_catalog.pg_table_is_visible(c2.oid)"\
" and c2.relispartition = 'true'"
+#define Query_for_list_of_cursors \
+" SELECT pg_catalog.quote_ident(name) "\
+" FROM pg_catalog.pg_cursors "\
+" WHERE substring(pg_catalog.quote_ident(name),1,%d)='%s'"
+
/*
* These object types were introduced later than our support cutoff of
* server version 7.4. We use the VersionedQuery infrastructure so that
@@ -2284,6 +2289,10 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH_VERSIONED_SCHEMA_QUERY(Query_for_list_of_procedures, NULL);
else if (Matches("CALL", MatchAny))
COMPLETE_WITH("(");
+/* CLOSE */
+ else if (Matches("CLOSE"))
+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors
+ " UNION SELECT 'ALL'");
/* CLUSTER */
else if (Matches("CLUSTER"))
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_clusterables, "UNION SELECT 'VERBOSE'");
@@ -3005,8 +3014,43 @@ psql_completion(const char *text, int start, int end)
else if (Matches("DECLARE", MatchAny))
COMPLETE_WITH("BINARY", "INSENSITIVE", "SCROLL", "NO SCROLL",
"CURSOR");
+ else if (Matches("DECLARE", MatchAny, "BINARY"))
+ COMPLETE_WITH("INSENSITIVE", "SCROLL", "NO SCROLL", "CURSOR");
+ else if (Matches("DECLARE", MatchAny, "INSENSITIVE"))
+ COMPLETE_WITH("BINARY", "SCROLL", "NO SCROLL", "CURSOR");
+ else if (Matches("DECLARE", MatchAny, "SCROLL") ||
+ Matches("DECLARE", MatchAny, "NO", "SCROLL"))
+ COMPLETE_WITH("BINARY", "INSENSITIVE", "CURSOR");
+ else if (Matches("DECLARE", MatchAny, "BINARY", "INSENSITIVE") ||
+ Matches("DECLARE", MatchAny, "INSENSITIVE", "BINARY"))
+ COMPLETE_WITH("SCROLL", "NO SCROLL", "CURSOR");
+ else if (Matches("DECLARE", MatchAny, "BINARY", "SCROLL") ||
+ Matches("DECLARE", MatchAny, "BINARY", "NO", "SCROLL") ||
+ Matches("DECLARE", MatchAny, "SCROLL", "BINARY") ||
+ Matches("DECLARE", MatchAny, "NO", "SCROLL", "BINARY"))
+ COMPLETE_WITH("INSENSITIVE", "CURSOR");
+ else if (Matches("DECLARE", MatchAny, "INSENSITIVE", "SCROLL") ||
+ Matches("DECLARE", MatchAny, "INSENSITIVE", "NO", "SCROLL") ||
+ Matches("DECLARE", MatchAny, "SCROLL", "INSENSITIVE") ||
+ Matches("DECLARE", MatchAny, "NO", "SCROLL", "INSENSITIVE"))
+ COMPLETE_WITH("BINARY", "CURSOR");
+ else if (Matches("DECLARE", MatchAny, "BINARY", "INSENSITIVE", "SCROLL") ||
+ Matches("DECLARE", MatchAny, "BINARY", "INSENSITIVE", "NO", "SCROLL") ||
+ Matches("DECLARE", MatchAny, "BINARY", "SCROLL", "INSENSITIVE") ||
+ Matches("DECLARE", MatchAny, "BINARY", "NO", "SCROLL", "INSENSITIVE") ||
+ Matches("DECLARE", MatchAny, "INSENSITIVE", "BINARY", "SCROLL") ||
+ Matches("DECLARE", MatchAny, "INSENSITIVE", "BINARY", "NO", "SCROLL") ||
+ Matches("DECLARE", MatchAny, "INSENSITIVE", "SCROLL", "BINARY") ||
+ Matches("DECLARE", MatchAny, "INSENSITIVE", "NO", "SCROLL", "BINARY") ||
+ Matches("DECLARE", MatchAny, "SCROLL", "BINARY", "INSENSITIVE") ||
+ Matches("DECLARE", MatchAny, "NO", "SCROLL", "BINARY", "INSENSITIVE") ||
+ Matches("DECLARE", MatchAny, "SCROLL", "INSENSITIVE", "BINARY") ||
+ Matches("DECLARE", MatchAny, "NO", "SCROLL", "INSENSITIVE", "BINARY"))
+ COMPLETE_WITH("CURSOR");
else if (HeadMatches("DECLARE") && TailMatches("CURSOR"))
COMPLETE_WITH("WITH HOLD", "WITHOUT HOLD", "FOR");
+ else if (HeadMatches("DECLARE") && TailMatches("HOLD"))
+ COMPLETE_WITH("FOR");
/* DELETE --- can be inside EXPLAIN, RULE, etc */
/* ... despite which, only complete DELETE with FROM at start of line */
@@ -3167,15 +3211,31 @@ psql_completion(const char *text, int start, int end)
/*
* Complete FETCH with one of ABSOLUTE, BACKWARD, FORWARD, RELATIVE, ALL,
- * NEXT, PRIOR, FIRST, LAST
+ * NEXT, PRIOR, FIRST, LAST, FROM, IN, and a list of cursors
*/
else if (Matches("FETCH|MOVE"))
- COMPLETE_WITH("ABSOLUTE", "BACKWARD", "FORWARD", "RELATIVE",
- "ALL", "NEXT", "PRIOR", "FIRST", "LAST");
+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors
+ " UNION SELECT 'ABSOLUTE'"
+ " UNION SELECT 'BACKWARD'"
+ " UNION SELECT 'FORWARD'"
+ " UNION SELECT 'RELATIVE'"
+ " UNION SELECT 'ALL'"
+ " UNION SELECT 'NEXT'"
+ " UNION SELECT 'PRIOR'"
+ " UNION SELECT 'FIRST'"
+ " UNION SELECT 'LAST'"
+ " UNION SELECT 'FROM'"
+ " UNION SELECT 'IN'");
- /* Complete FETCH BACKWARD or FORWARD with one of ALL, FROM, IN */
+ /*
+ * Complete FETCH BACKWARD or FORWARD with one of ALL, FROM, IN,
+ * and a list of cursors
+ */
else if (Matches("FETCH|MOVE", "BACKWARD|FORWARD"))
- COMPLETE_WITH("ALL", "FROM", "IN");
+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors
+ " UNION SELECT 'ALL'"
+ " UNION SELECT 'FROM'"
+ " UNION SELECT 'IN'");
/*
* Complete FETCH <direction> with "FROM" or "IN". These are equivalent,
@@ -3185,7 +3245,13 @@ psql_completion(const char *text, int start, int end)
else if (Matches("FETCH|MOVE", "ABSOLUTE|BACKWARD|FORWARD|RELATIVE",
MatchAnyExcept("FROM|IN")) ||
Matches("FETCH|MOVE", "ALL|NEXT|PRIOR|FIRST|LAST"))
- COMPLETE_WITH("FROM", "IN");
+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors
+ " UNION SELECT 'FROM'"
+ " UNION SELECT 'IN'");
+ /* Complete FETCH <direction> "FROM" or "IN" with a list of cursors */
+ else if (HeadMatches("FETCH|MOVE") &&
+ TailMatches("FROM|IN"))
+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors);
/* FOREIGN DATA WRAPPER */
/* applies in ALTER/DROP FDW and in CREATE SERVER */
* Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion
2020-12-09 03:57 [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-04 11:56 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-05 06:02 ` RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-05 09:08 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@oss.nttdata.com>
2021-01-06 02:13 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-06 02:33 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@oss.nttdata.com>
2021-01-06 06:36 ` RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
@ 2021-01-07 01:01 ` Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-07 01:59 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@oss.nttdata.com>
0 siblings, 1 reply; 22+ messages in thread
From: Masahiko Sawada @ 2021-01-07 01:01 UTC (permalink / raw)
To: Shinya11.Kato@nttdata.com; +Cc: Fujii Masao <masao.fujii@oss.nttdata.com>; pgsql-hackers
On Wed, Jan 6, 2021 at 3:37 PM <Shinya11.Kato@nttdata.com> wrote:
>
> >+#define Query_for_list_of_cursors \
> >+" SELECT name FROM pg_cursors"\
> >
> >This query should be the following?
> >
> >" SELECT pg_catalog.quote_ident(name) "\
> >" FROM pg_catalog.pg_cursors "\
> >" WHERE substring(pg_catalog.quote_ident(name),1,%d)='%s'"
> >
> >+/* CLOSE */
> >+ else if (Matches("CLOSE"))
> >+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors
> >+ " UNION ALL SELECT 'ALL'");
> >
> >"UNION ALL" should be "UNION"?
>
> Thank you for your advice, and I corrected them.
>
> >> + COMPLETE_WITH_QUERY(Query_for_list_of_cursors
> >> + " UNION SELECT 'ABSOLUTE'"
> >> + " UNION SELECT 'BACKWARD'"
> >> + " UNION SELECT 'FORWARD'"
> >> + " UNION SELECT 'RELATIVE'"
> >> + " UNION SELECT 'ALL'"
> >> + " UNION SELECT 'NEXT'"
> >> + " UNION SELECT 'PRIOR'"
> >> + " UNION SELECT 'FIRST'"
> >> + " UNION SELECT 'LAST'"
> >> + " UNION SELECT 'FROM'"
> >> + " UNION SELECT 'IN'");
> >>
> >> This change makes psql unexpectedly output "FROM" and "IN" just after "FETCH".
> >
> >I think "FROM" and "IN" can be placed just after "FETCH". According to
> >the documentation, the direction can be empty. For instance, we can do
> >like:
>
> Thank you!
>
> >I've attached the patch improving the tab completion for DECLARE as an
> >example. What do you think?
> >
> >BTW according to the documentation, the options of DECLARE statement
> >(BINARY, INSENSITIVE, SCROLL, and NO SCROLL) are order-sensitive.
> >
> >DECLARE name [ BINARY ] [ INSENSITIVE ] [ [ NO ] SCROLL ]
> > CURSOR [ { WITH | WITHOUT } HOLD ] FOR query
> >
> >But I realized that these options are actually order-insensitive. For
> >instance, we can declare a cursor like:
> >
> >=# declare abc scroll binary cursor for select * from pg_class;
> >DECLARE CURSOR
> >
> >The both parser code and documentation has been unchanged from 2003.
> >Is it a documentation bug?
>
> Thank you for your patch, and it is good.
> I cannot find the description "(BINARY, INSENSITIVE, SCROLL, and NO SCROLL) are order-sensitive."
> I saw "The key words BINARY, INSENSITIVE, and SCROLL can appear in any order." , according to the documentation.
Thanks, you're right. I missed that sentence. I still don't think the
syntax of DECLARE statement in the documentation doesn't match its
implementation but I agree that it's order-insensitive.
> I made a new patch, but the amount of codes was large due to order-insensitive.
Thank you for updating the patch!
Yeah, I'm also afraid a bit that conditions will exponentially
increase when a new option is added to DECLARE statement in the
future. Looking at the parser code for DECLARE statement, we can put
the same options multiple times (that's also why I don't think it
matches). For instance,
postgres(1:44758)=# begin;
BEGIN
postgres(1:44758)=# declare test binary binary binary cursor for
select * from pg_class;
DECLARE CURSOR
So how about simplify the above code as follows?
@@ -3005,8 +3014,23 @@ psql_completion(const char *text, int start, int end)
else if (Matches("DECLARE", MatchAny))
COMPLETE_WITH("BINARY", "INSENSITIVE", "SCROLL", "NO SCROLL",
"CURSOR");
+ /*
+ * Complete DECLARE <name> with one of BINARY, INSENSITIVE, SCROLL,
+ * NO SCROLL, and CURSOR. The tail doesn't match any keywords for
+ * DECLARE, assume we want options.
+ */
+ else if (HeadMatches("DECLARE", MatchAny, "*") &&
+ TailMatches(MatchAnyExcept("CURSOR|WITH|WITHOUT|HOLD|FOR")))
+ COMPLETE_WITH("BINARY", "INSENSITIVE", "SCROLL", "NO SCROLL",
+ "CURSOR");
+ /*
+ * Complete DECLARE <name> ... CURSOR with one of WITH HOLD, WITHOUT HOLD,
+ * and FOR.
+ */
else if (HeadMatches("DECLARE") && TailMatches("CURSOR"))
COMPLETE_WITH("WITH HOLD", "WITHOUT HOLD", "FOR");
+ else if (HeadMatches("DECLARE") && TailMatches("HOLD"))
+ COMPLETE_WITH("FOR");
Regards,
--
Masahiko Sawada
EnterpriseDB: https://www.enterprisedb.com/
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion
2020-12-09 03:57 [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-04 11:56 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-05 06:02 ` RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-05 09:08 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@oss.nttdata.com>
2021-01-06 02:13 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-06 02:33 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@oss.nttdata.com>
2021-01-06 06:36 ` RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-07 01:01 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
@ 2021-01-07 01:59 ` Fujii Masao <masao.fujii@oss.nttdata.com>
2021-01-07 03:42 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
0 siblings, 1 reply; 22+ messages in thread
From: Fujii Masao @ 2021-01-07 01:59 UTC (permalink / raw)
To: Masahiko Sawada <sawada.mshk@gmail.com>; Shinya11.Kato@nttdata.com; +Cc: pgsql-hackers
On 2021/01/07 10:01, Masahiko Sawada wrote:
> On Wed, Jan 6, 2021 at 3:37 PM <Shinya11.Kato@nttdata.com> wrote:
>>
>>> +#define Query_for_list_of_cursors \
>>> +" SELECT name FROM pg_cursors"\
>>>
>>> This query should be the following?
>>>
>>> " SELECT pg_catalog.quote_ident(name) "\
>>> " FROM pg_catalog.pg_cursors "\
>>> " WHERE substring(pg_catalog.quote_ident(name),1,%d)='%s'"
>>>
>>> +/* CLOSE */
>>> + else if (Matches("CLOSE"))
>>> + COMPLETE_WITH_QUERY(Query_for_list_of_cursors
>>> + " UNION ALL SELECT 'ALL'");
>>>
>>> "UNION ALL" should be "UNION"?
>>
>> Thank you for your advice, and I corrected them.
>>
>>>> + COMPLETE_WITH_QUERY(Query_for_list_of_cursors
>>>> + " UNION SELECT 'ABSOLUTE'"
>>>> + " UNION SELECT 'BACKWARD'"
>>>> + " UNION SELECT 'FORWARD'"
>>>> + " UNION SELECT 'RELATIVE'"
>>>> + " UNION SELECT 'ALL'"
>>>> + " UNION SELECT 'NEXT'"
>>>> + " UNION SELECT 'PRIOR'"
>>>> + " UNION SELECT 'FIRST'"
>>>> + " UNION SELECT 'LAST'"
>>>> + " UNION SELECT 'FROM'"
>>>> + " UNION SELECT 'IN'");
>>>>
>>>> This change makes psql unexpectedly output "FROM" and "IN" just after "FETCH".
>>>
>>> I think "FROM" and "IN" can be placed just after "FETCH". According to
>>> the documentation, the direction can be empty. For instance, we can do
>>> like:
>>
>> Thank you!
>>
>>> I've attached the patch improving the tab completion for DECLARE as an
>>> example. What do you think?
>>>
>>> BTW according to the documentation, the options of DECLARE statement
>>> (BINARY, INSENSITIVE, SCROLL, and NO SCROLL) are order-sensitive.
>>>
>>> DECLARE name [ BINARY ] [ INSENSITIVE ] [ [ NO ] SCROLL ]
>>> CURSOR [ { WITH | WITHOUT } HOLD ] FOR query
>>>
>>> But I realized that these options are actually order-insensitive. For
>>> instance, we can declare a cursor like:
>>>
>>> =# declare abc scroll binary cursor for select * from pg_class;
>>> DECLARE CURSOR
>>>
>>> The both parser code and documentation has been unchanged from 2003.
>>> Is it a documentation bug?
>>
>> Thank you for your patch, and it is good.
>> I cannot find the description "(BINARY, INSENSITIVE, SCROLL, and NO SCROLL) are order-sensitive."
>> I saw "The key words BINARY, INSENSITIVE, and SCROLL can appear in any order." , according to the documentation.
>
> Thanks, you're right. I missed that sentence. I still don't think the
> syntax of DECLARE statement in the documentation doesn't match its
> implementation but I agree that it's order-insensitive.
>
>> I made a new patch, but the amount of codes was large due to order-insensitive.
>
> Thank you for updating the patch!
>
> Yeah, I'm also afraid a bit that conditions will exponentially
> increase when a new option is added to DECLARE statement in the
> future. Looking at the parser code for DECLARE statement, we can put
> the same options multiple times (that's also why I don't think it
> matches). For instance,
>
> postgres(1:44758)=# begin;
> BEGIN
> postgres(1:44758)=# declare test binary binary binary cursor for
> select * from pg_class;
> DECLARE CURSOR
>
> So how about simplify the above code as follows?
>
> @@ -3005,8 +3014,23 @@ psql_completion(const char *text, int start, int end)
> else if (Matches("DECLARE", MatchAny))
> COMPLETE_WITH("BINARY", "INSENSITIVE", "SCROLL", "NO SCROLL",
> "CURSOR");
> + /*
> + * Complete DECLARE <name> with one of BINARY, INSENSITIVE, SCROLL,
> + * NO SCROLL, and CURSOR. The tail doesn't match any keywords for
> + * DECLARE, assume we want options.
> + */
> + else if (HeadMatches("DECLARE", MatchAny, "*") &&
> + TailMatches(MatchAnyExcept("CURSOR|WITH|WITHOUT|HOLD|FOR")))
> + COMPLETE_WITH("BINARY", "INSENSITIVE", "SCROLL", "NO SCROLL",
> + "CURSOR");
This change seems to cause "DECLARE ... CURSOR FOR SELECT <tab>" to
unexpectedly output BINARY, INSENSITIVE, etc.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion
2020-12-09 03:57 [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-04 11:56 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-05 06:02 ` RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-05 09:08 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@oss.nttdata.com>
2021-01-06 02:13 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-06 02:33 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@oss.nttdata.com>
2021-01-06 06:36 ` RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-07 01:01 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-07 01:59 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@oss.nttdata.com>
@ 2021-01-07 03:42 ` Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-07 04:30 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@oss.nttdata.com>
0 siblings, 1 reply; 22+ messages in thread
From: Masahiko Sawada @ 2021-01-07 03:42 UTC (permalink / raw)
To: Fujii Masao <masao.fujii@oss.nttdata.com>; +Cc: pgsql-hackers
On Thu, Jan 7, 2021 at 10:59 AM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>
>
>
> On 2021/01/07 10:01, Masahiko Sawada wrote:
> > On Wed, Jan 6, 2021 at 3:37 PM <Shinya11.Kato@nttdata.com> wrote:
> >>
> >>> +#define Query_for_list_of_cursors \
> >>> +" SELECT name FROM pg_cursors"\
> >>>
> >>> This query should be the following?
> >>>
> >>> " SELECT pg_catalog.quote_ident(name) "\
> >>> " FROM pg_catalog.pg_cursors "\
> >>> " WHERE substring(pg_catalog.quote_ident(name),1,%d)='%s'"
> >>>
> >>> +/* CLOSE */
> >>> + else if (Matches("CLOSE"))
> >>> + COMPLETE_WITH_QUERY(Query_for_list_of_cursors
> >>> + " UNION ALL SELECT 'ALL'");
> >>>
> >>> "UNION ALL" should be "UNION"?
> >>
> >> Thank you for your advice, and I corrected them.
> >>
> >>>> + COMPLETE_WITH_QUERY(Query_for_list_of_cursors
> >>>> + " UNION SELECT 'ABSOLUTE'"
> >>>> + " UNION SELECT 'BACKWARD'"
> >>>> + " UNION SELECT 'FORWARD'"
> >>>> + " UNION SELECT 'RELATIVE'"
> >>>> + " UNION SELECT 'ALL'"
> >>>> + " UNION SELECT 'NEXT'"
> >>>> + " UNION SELECT 'PRIOR'"
> >>>> + " UNION SELECT 'FIRST'"
> >>>> + " UNION SELECT 'LAST'"
> >>>> + " UNION SELECT 'FROM'"
> >>>> + " UNION SELECT 'IN'");
> >>>>
> >>>> This change makes psql unexpectedly output "FROM" and "IN" just after "FETCH".
> >>>
> >>> I think "FROM" and "IN" can be placed just after "FETCH". According to
> >>> the documentation, the direction can be empty. For instance, we can do
> >>> like:
> >>
> >> Thank you!
> >>
> >>> I've attached the patch improving the tab completion for DECLARE as an
> >>> example. What do you think?
> >>>
> >>> BTW according to the documentation, the options of DECLARE statement
> >>> (BINARY, INSENSITIVE, SCROLL, and NO SCROLL) are order-sensitive.
> >>>
> >>> DECLARE name [ BINARY ] [ INSENSITIVE ] [ [ NO ] SCROLL ]
> >>> CURSOR [ { WITH | WITHOUT } HOLD ] FOR query
> >>>
> >>> But I realized that these options are actually order-insensitive. For
> >>> instance, we can declare a cursor like:
> >>>
> >>> =# declare abc scroll binary cursor for select * from pg_class;
> >>> DECLARE CURSOR
> >>>
> >>> The both parser code and documentation has been unchanged from 2003.
> >>> Is it a documentation bug?
> >>
> >> Thank you for your patch, and it is good.
> >> I cannot find the description "(BINARY, INSENSITIVE, SCROLL, and NO SCROLL) are order-sensitive."
> >> I saw "The key words BINARY, INSENSITIVE, and SCROLL can appear in any order." , according to the documentation.
> >
> > Thanks, you're right. I missed that sentence. I still don't think the
> > syntax of DECLARE statement in the documentation doesn't match its
> > implementation but I agree that it's order-insensitive.
> >
> >> I made a new patch, but the amount of codes was large due to order-insensitive.
> >
> > Thank you for updating the patch!
> >
> > Yeah, I'm also afraid a bit that conditions will exponentially
> > increase when a new option is added to DECLARE statement in the
> > future. Looking at the parser code for DECLARE statement, we can put
> > the same options multiple times (that's also why I don't think it
> > matches). For instance,
> >
> > postgres(1:44758)=# begin;
> > BEGIN
> > postgres(1:44758)=# declare test binary binary binary cursor for
> > select * from pg_class;
> > DECLARE CURSOR
> >
> > So how about simplify the above code as follows?
> >
> > @@ -3005,8 +3014,23 @@ psql_completion(const char *text, int start, int end)
> > else if (Matches("DECLARE", MatchAny))
> > COMPLETE_WITH("BINARY", "INSENSITIVE", "SCROLL", "NO SCROLL",
> > "CURSOR");
> > + /*
> > + * Complete DECLARE <name> with one of BINARY, INSENSITIVE, SCROLL,
> > + * NO SCROLL, and CURSOR. The tail doesn't match any keywords for
> > + * DECLARE, assume we want options.
> > + */
> > + else if (HeadMatches("DECLARE", MatchAny, "*") &&
> > + TailMatches(MatchAnyExcept("CURSOR|WITH|WITHOUT|HOLD|FOR")))
> > + COMPLETE_WITH("BINARY", "INSENSITIVE", "SCROLL", "NO SCROLL",
> > + "CURSOR");
>
> This change seems to cause "DECLARE ... CURSOR FOR SELECT <tab>" to
> unexpectedly output BINARY, INSENSITIVE, etc.
Indeed. Is the following not complete but much better?
@@ -3002,11 +3011,18 @@ psql_completion(const char *text, int start, int end)
" UNION SELECT 'ALL'");
/* DECLARE */
- else if (Matches("DECLARE", MatchAny))
- COMPLETE_WITH("BINARY", "INSENSITIVE", "SCROLL", "NO SCROLL",
- "CURSOR");
+ /*
+ * Complete DECLARE <name> with one of BINARY, INSENSITIVE, SCROLL,
+ * NO SCROLL, and CURSOR. If the tail is any one of options, assume we
+ * still want options.
+ */
+ else if (Matches("DECLARE", MatchAny) ||
+ TailMatches("BINARY|INSENSITIVE|SCROLL|NO"))
+ COMPLETE_WITH("BINARY", "INSENSITIVE", "SCROLL", "NO SCROLL", "CURSOR");
else if (HeadMatches("DECLARE") && TailMatches("CURSOR"))
COMPLETE_WITH("WITH HOLD", "WITHOUT HOLD", "FOR");
+ else if (HeadMatches("DECLARE") && TailMatches("HOLD"))
+ COMPLETE_WITH("FOR");
Regards,
--
Masahiko Sawada
EnterpriseDB: https://www.enterprisedb.com/
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion
2020-12-09 03:57 [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-04 11:56 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-05 06:02 ` RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-05 09:08 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@oss.nttdata.com>
2021-01-06 02:13 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-06 02:33 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@oss.nttdata.com>
2021-01-06 06:36 ` RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-07 01:01 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-07 01:59 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@oss.nttdata.com>
2021-01-07 03:42 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
@ 2021-01-07 04:30 ` Fujii Masao <masao.fujii@oss.nttdata.com>
2021-01-07 06:53 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
0 siblings, 1 reply; 22+ messages in thread
From: Fujii Masao @ 2021-01-07 04:30 UTC (permalink / raw)
To: Masahiko Sawada <sawada.mshk@gmail.com>; +Cc: pgsql-hackers
On 2021/01/07 12:42, Masahiko Sawada wrote:
> On Thu, Jan 7, 2021 at 10:59 AM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>>
>>
>>
>> On 2021/01/07 10:01, Masahiko Sawada wrote:
>>> On Wed, Jan 6, 2021 at 3:37 PM <Shinya11.Kato@nttdata.com> wrote:
>>>>
>>>>> +#define Query_for_list_of_cursors \
>>>>> +" SELECT name FROM pg_cursors"\
>>>>>
>>>>> This query should be the following?
>>>>>
>>>>> " SELECT pg_catalog.quote_ident(name) "\
>>>>> " FROM pg_catalog.pg_cursors "\
>>>>> " WHERE substring(pg_catalog.quote_ident(name),1,%d)='%s'"
>>>>>
>>>>> +/* CLOSE */
>>>>> + else if (Matches("CLOSE"))
>>>>> + COMPLETE_WITH_QUERY(Query_for_list_of_cursors
>>>>> + " UNION ALL SELECT 'ALL'");
>>>>>
>>>>> "UNION ALL" should be "UNION"?
>>>>
>>>> Thank you for your advice, and I corrected them.
>>>>
>>>>>> + COMPLETE_WITH_QUERY(Query_for_list_of_cursors
>>>>>> + " UNION SELECT 'ABSOLUTE'"
>>>>>> + " UNION SELECT 'BACKWARD'"
>>>>>> + " UNION SELECT 'FORWARD'"
>>>>>> + " UNION SELECT 'RELATIVE'"
>>>>>> + " UNION SELECT 'ALL'"
>>>>>> + " UNION SELECT 'NEXT'"
>>>>>> + " UNION SELECT 'PRIOR'"
>>>>>> + " UNION SELECT 'FIRST'"
>>>>>> + " UNION SELECT 'LAST'"
>>>>>> + " UNION SELECT 'FROM'"
>>>>>> + " UNION SELECT 'IN'");
>>>>>>
>>>>>> This change makes psql unexpectedly output "FROM" and "IN" just after "FETCH".
>>>>>
>>>>> I think "FROM" and "IN" can be placed just after "FETCH". According to
>>>>> the documentation, the direction can be empty. For instance, we can do
>>>>> like:
>>>>
>>>> Thank you!
>>>>
>>>>> I've attached the patch improving the tab completion for DECLARE as an
>>>>> example. What do you think?
>>>>>
>>>>> BTW according to the documentation, the options of DECLARE statement
>>>>> (BINARY, INSENSITIVE, SCROLL, and NO SCROLL) are order-sensitive.
>>>>>
>>>>> DECLARE name [ BINARY ] [ INSENSITIVE ] [ [ NO ] SCROLL ]
>>>>> CURSOR [ { WITH | WITHOUT } HOLD ] FOR query
>>>>>
>>>>> But I realized that these options are actually order-insensitive. For
>>>>> instance, we can declare a cursor like:
>>>>>
>>>>> =# declare abc scroll binary cursor for select * from pg_class;
>>>>> DECLARE CURSOR
>>>>>
>>>>> The both parser code and documentation has been unchanged from 2003.
>>>>> Is it a documentation bug?
>>>>
>>>> Thank you for your patch, and it is good.
>>>> I cannot find the description "(BINARY, INSENSITIVE, SCROLL, and NO SCROLL) are order-sensitive."
>>>> I saw "The key words BINARY, INSENSITIVE, and SCROLL can appear in any order." , according to the documentation.
>>>
>>> Thanks, you're right. I missed that sentence. I still don't think the
>>> syntax of DECLARE statement in the documentation doesn't match its
>>> implementation but I agree that it's order-insensitive.
>>>
>>>> I made a new patch, but the amount of codes was large due to order-insensitive.
>>>
>>> Thank you for updating the patch!
>>>
>>> Yeah, I'm also afraid a bit that conditions will exponentially
>>> increase when a new option is added to DECLARE statement in the
>>> future. Looking at the parser code for DECLARE statement, we can put
>>> the same options multiple times (that's also why I don't think it
>>> matches). For instance,
>>>
>>> postgres(1:44758)=# begin;
>>> BEGIN
>>> postgres(1:44758)=# declare test binary binary binary cursor for
>>> select * from pg_class;
>>> DECLARE CURSOR
>>>
>>> So how about simplify the above code as follows?
>>>
>>> @@ -3005,8 +3014,23 @@ psql_completion(const char *text, int start, int end)
>>> else if (Matches("DECLARE", MatchAny))
>>> COMPLETE_WITH("BINARY", "INSENSITIVE", "SCROLL", "NO SCROLL",
>>> "CURSOR");
>>> + /*
>>> + * Complete DECLARE <name> with one of BINARY, INSENSITIVE, SCROLL,
>>> + * NO SCROLL, and CURSOR. The tail doesn't match any keywords for
>>> + * DECLARE, assume we want options.
>>> + */
>>> + else if (HeadMatches("DECLARE", MatchAny, "*") &&
>>> + TailMatches(MatchAnyExcept("CURSOR|WITH|WITHOUT|HOLD|FOR")))
>>> + COMPLETE_WITH("BINARY", "INSENSITIVE", "SCROLL", "NO SCROLL",
>>> + "CURSOR");
>>
>> This change seems to cause "DECLARE ... CURSOR FOR SELECT <tab>" to
>> unexpectedly output BINARY, INSENSITIVE, etc.
>
> Indeed. Is the following not complete but much better?
Yes, I think that's better.
>
> @@ -3002,11 +3011,18 @@ psql_completion(const char *text, int start, int end)
> " UNION SELECT 'ALL'");
>
> /* DECLARE */
> - else if (Matches("DECLARE", MatchAny))
> - COMPLETE_WITH("BINARY", "INSENSITIVE", "SCROLL", "NO SCROLL",
> - "CURSOR");
> + /*
> + * Complete DECLARE <name> with one of BINARY, INSENSITIVE, SCROLL,
> + * NO SCROLL, and CURSOR. If the tail is any one of options, assume we
> + * still want options.
> + */
> + else if (Matches("DECLARE", MatchAny) ||
> + TailMatches("BINARY|INSENSITIVE|SCROLL|NO"))
> + COMPLETE_WITH("BINARY", "INSENSITIVE", "SCROLL", "NO SCROLL", "CURSOR");
This change seems to cause "DECLARE ... NO <tab>" to unexpectedly output
"NO SCROLL". Also this change seems to cause "COPY ... (FORMAT BINARY <tab>"
to unexpectedly output BINARY, CURSOR, etc.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion
2020-12-09 03:57 [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-04 11:56 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-05 06:02 ` RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-05 09:08 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@oss.nttdata.com>
2021-01-06 02:13 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-06 02:33 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@oss.nttdata.com>
2021-01-06 06:36 ` RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-07 01:01 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-07 01:59 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@oss.nttdata.com>
2021-01-07 03:42 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-07 04:30 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@oss.nttdata.com>
@ 2021-01-07 06:53 ` Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-07 08:28 ` RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
0 siblings, 1 reply; 22+ messages in thread
From: Masahiko Sawada @ 2021-01-07 06:53 UTC (permalink / raw)
To: Fujii Masao <masao.fujii@oss.nttdata.com>; +Cc: pgsql-hackers
On Thu, Jan 7, 2021 at 1:30 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>
>
>
> On 2021/01/07 12:42, Masahiko Sawada wrote:
> > On Thu, Jan 7, 2021 at 10:59 AM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
> >>
> >>
> >>
> >> On 2021/01/07 10:01, Masahiko Sawada wrote:
> >>> On Wed, Jan 6, 2021 at 3:37 PM <Shinya11.Kato@nttdata.com> wrote:
> >>>>
> >>>>> +#define Query_for_list_of_cursors \
> >>>>> +" SELECT name FROM pg_cursors"\
> >>>>>
> >>>>> This query should be the following?
> >>>>>
> >>>>> " SELECT pg_catalog.quote_ident(name) "\
> >>>>> " FROM pg_catalog.pg_cursors "\
> >>>>> " WHERE substring(pg_catalog.quote_ident(name),1,%d)='%s'"
> >>>>>
> >>>>> +/* CLOSE */
> >>>>> + else if (Matches("CLOSE"))
> >>>>> + COMPLETE_WITH_QUERY(Query_for_list_of_cursors
> >>>>> + " UNION ALL SELECT 'ALL'");
> >>>>>
> >>>>> "UNION ALL" should be "UNION"?
> >>>>
> >>>> Thank you for your advice, and I corrected them.
> >>>>
> >>>>>> + COMPLETE_WITH_QUERY(Query_for_list_of_cursors
> >>>>>> + " UNION SELECT 'ABSOLUTE'"
> >>>>>> + " UNION SELECT 'BACKWARD'"
> >>>>>> + " UNION SELECT 'FORWARD'"
> >>>>>> + " UNION SELECT 'RELATIVE'"
> >>>>>> + " UNION SELECT 'ALL'"
> >>>>>> + " UNION SELECT 'NEXT'"
> >>>>>> + " UNION SELECT 'PRIOR'"
> >>>>>> + " UNION SELECT 'FIRST'"
> >>>>>> + " UNION SELECT 'LAST'"
> >>>>>> + " UNION SELECT 'FROM'"
> >>>>>> + " UNION SELECT 'IN'");
> >>>>>>
> >>>>>> This change makes psql unexpectedly output "FROM" and "IN" just after "FETCH".
> >>>>>
> >>>>> I think "FROM" and "IN" can be placed just after "FETCH". According to
> >>>>> the documentation, the direction can be empty. For instance, we can do
> >>>>> like:
> >>>>
> >>>> Thank you!
> >>>>
> >>>>> I've attached the patch improving the tab completion for DECLARE as an
> >>>>> example. What do you think?
> >>>>>
> >>>>> BTW according to the documentation, the options of DECLARE statement
> >>>>> (BINARY, INSENSITIVE, SCROLL, and NO SCROLL) are order-sensitive.
> >>>>>
> >>>>> DECLARE name [ BINARY ] [ INSENSITIVE ] [ [ NO ] SCROLL ]
> >>>>> CURSOR [ { WITH | WITHOUT } HOLD ] FOR query
> >>>>>
> >>>>> But I realized that these options are actually order-insensitive. For
> >>>>> instance, we can declare a cursor like:
> >>>>>
> >>>>> =# declare abc scroll binary cursor for select * from pg_class;
> >>>>> DECLARE CURSOR
> >>>>>
> >>>>> The both parser code and documentation has been unchanged from 2003.
> >>>>> Is it a documentation bug?
> >>>>
> >>>> Thank you for your patch, and it is good.
> >>>> I cannot find the description "(BINARY, INSENSITIVE, SCROLL, and NO SCROLL) are order-sensitive."
> >>>> I saw "The key words BINARY, INSENSITIVE, and SCROLL can appear in any order." , according to the documentation.
> >>>
> >>> Thanks, you're right. I missed that sentence. I still don't think the
> >>> syntax of DECLARE statement in the documentation doesn't match its
> >>> implementation but I agree that it's order-insensitive.
> >>>
> >>>> I made a new patch, but the amount of codes was large due to order-insensitive.
> >>>
> >>> Thank you for updating the patch!
> >>>
> >>> Yeah, I'm also afraid a bit that conditions will exponentially
> >>> increase when a new option is added to DECLARE statement in the
> >>> future. Looking at the parser code for DECLARE statement, we can put
> >>> the same options multiple times (that's also why I don't think it
> >>> matches). For instance,
> >>>
> >>> postgres(1:44758)=# begin;
> >>> BEGIN
> >>> postgres(1:44758)=# declare test binary binary binary cursor for
> >>> select * from pg_class;
> >>> DECLARE CURSOR
> >>>
> >>> So how about simplify the above code as follows?
> >>>
> >>> @@ -3005,8 +3014,23 @@ psql_completion(const char *text, int start, int end)
> >>> else if (Matches("DECLARE", MatchAny))
> >>> COMPLETE_WITH("BINARY", "INSENSITIVE", "SCROLL", "NO SCROLL",
> >>> "CURSOR");
> >>> + /*
> >>> + * Complete DECLARE <name> with one of BINARY, INSENSITIVE, SCROLL,
> >>> + * NO SCROLL, and CURSOR. The tail doesn't match any keywords for
> >>> + * DECLARE, assume we want options.
> >>> + */
> >>> + else if (HeadMatches("DECLARE", MatchAny, "*") &&
> >>> + TailMatches(MatchAnyExcept("CURSOR|WITH|WITHOUT|HOLD|FOR")))
> >>> + COMPLETE_WITH("BINARY", "INSENSITIVE", "SCROLL", "NO SCROLL",
> >>> + "CURSOR");
> >>
> >> This change seems to cause "DECLARE ... CURSOR FOR SELECT <tab>" to
> >> unexpectedly output BINARY, INSENSITIVE, etc.
> >
> > Indeed. Is the following not complete but much better?
>
> Yes, I think that's better.
>
> >
> > @@ -3002,11 +3011,18 @@ psql_completion(const char *text, int start, int end)
> > " UNION SELECT 'ALL'");
> >
> > /* DECLARE */
> > - else if (Matches("DECLARE", MatchAny))
> > - COMPLETE_WITH("BINARY", "INSENSITIVE", "SCROLL", "NO SCROLL",
> > - "CURSOR");
> > + /*
> > + * Complete DECLARE <name> with one of BINARY, INSENSITIVE, SCROLL,
> > + * NO SCROLL, and CURSOR. If the tail is any one of options, assume we
> > + * still want options.
> > + */
> > + else if (Matches("DECLARE", MatchAny) ||
> > + TailMatches("BINARY|INSENSITIVE|SCROLL|NO"))
> > + COMPLETE_WITH("BINARY", "INSENSITIVE", "SCROLL", "NO SCROLL", "CURSOR");
>
> This change seems to cause "DECLARE ... NO <tab>" to unexpectedly output
> "NO SCROLL". Also this change seems to cause "COPY ... (FORMAT BINARY <tab>"
> to unexpectedly output BINARY, CURSOR, etc.
Oops, I missed the HeadMatches(). Thank you for pointing this out.
I've attached the updated patch including Kato-san's changes. I
think the tab completion support for DECLARE added by this patch
works better.
Regards,
--
Masahiko Sawada
EnterpriseDB: https://www.enterprisedb.com/
Attachments:
fix_tab_complete_close_fetch_move_v4.patch (application/x-patch, 4.6K) download
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 9dcab0d2fa..5838319676 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -976,6 +976,11 @@ static const SchemaQuery Query_for_list_of_statistics = {
" and pg_catalog.pg_table_is_visible(c2.oid)"\
" and c2.relispartition = 'true'"
+#define Query_for_list_of_cursors \
+" SELECT pg_catalog.quote_ident(name) "\
+" FROM pg_catalog.pg_cursors "\
+" WHERE substring(pg_catalog.quote_ident(name),1,%d)='%s'"
+
/*
* These object types were introduced later than our support cutoff of
* server version 7.4. We use the VersionedQuery infrastructure so that
@@ -2284,6 +2289,10 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH_VERSIONED_SCHEMA_QUERY(Query_for_list_of_procedures, NULL);
else if (Matches("CALL", MatchAny))
COMPLETE_WITH("(");
+/* CLOSE */
+ else if (Matches("CLOSE"))
+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors
+ " UNION SELECT 'ALL'");
/* CLUSTER */
else if (Matches("CLUSTER"))
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_clusterables, "UNION SELECT 'VERBOSE'");
@@ -3002,11 +3011,37 @@ psql_completion(const char *text, int start, int end)
" UNION SELECT 'ALL'");
/* DECLARE */
+ /*
+ * Complete DECLARE <name> with one of BINARY, INSENSITIVE, SCROLL, NO SCROLL,
+ * and CURSOR.
+ */
else if (Matches("DECLARE", MatchAny))
COMPLETE_WITH("BINARY", "INSENSITIVE", "SCROLL", "NO SCROLL",
"CURSOR");
+ /* Complete with more options */
+ else if (HeadMatches("DECLARE", MatchAny, "BINARY|INSENSITIVE|SCROLL|NO") &&
+ TailMatches("BINARY|INSENSITIVE|SCROLL|NO"))
+ {
+ /* Complete DECLARE <name> [options] NO with SCROLL */
+ if (TailMatches("NO"))
+ COMPLETE_WITH("SCROLL");
+ /*
+ * Complete DECLARE <name> [options] with one of BINARY, INSENSITIVE, SCROLL,
+ * NO SCROLL, and CURSOR. If the tail is any one of options, assume we
+ * still want options.
+ */
+ else if (TailMatches("BINARY|INSENSITIVE|SCROLL"))
+ COMPLETE_WITH("BINARY", "INSENSITIVE", "SCROLL", "NO SCROLL", "CURSOR");
+ }
+ /* Complete DECLARE ... CURSOR with one of WITH HOLD, WITHOUT HOLD, and FOR */
else if (HeadMatches("DECLARE") && TailMatches("CURSOR"))
COMPLETE_WITH("WITH HOLD", "WITHOUT HOLD", "FOR");
+ /* Complete DECLARE ... CURSOR WITH|WITHOUT with HOLD */
+ else if (HeadMatches("DECLARE") && TailMatches("CURSOR", "WITH|WITHOUT"))
+ COMPLETE_WITH("HOLD");
+ /* Complete DECLARE ... CURSOR WITH|WITHOUT HOLD with FOR */
+ else if (HeadMatches("DECLARE") && TailMatches("CURSOR", "WITH|WITHOUT", "HOLD"))
+ COMPLETE_WITH("FOR");
/* DELETE --- can be inside EXPLAIN, RULE, etc */
/* ... despite which, only complete DELETE with FROM at start of line */
@@ -3167,15 +3202,31 @@ psql_completion(const char *text, int start, int end)
/*
* Complete FETCH with one of ABSOLUTE, BACKWARD, FORWARD, RELATIVE, ALL,
- * NEXT, PRIOR, FIRST, LAST
+ * NEXT, PRIOR, FIRST, LAST, FROM, IN, and a list of cursors
*/
else if (Matches("FETCH|MOVE"))
- COMPLETE_WITH("ABSOLUTE", "BACKWARD", "FORWARD", "RELATIVE",
- "ALL", "NEXT", "PRIOR", "FIRST", "LAST");
+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors
+ " UNION SELECT 'ABSOLUTE'"
+ " UNION SELECT 'BACKWARD'"
+ " UNION SELECT 'FORWARD'"
+ " UNION SELECT 'RELATIVE'"
+ " UNION SELECT 'ALL'"
+ " UNION SELECT 'NEXT'"
+ " UNION SELECT 'PRIOR'"
+ " UNION SELECT 'FIRST'"
+ " UNION SELECT 'LAST'"
+ " UNION SELECT 'FROM'"
+ " UNION SELECT 'IN'");
- /* Complete FETCH BACKWARD or FORWARD with one of ALL, FROM, IN */
+ /*
+ * Complete FETCH BACKWARD or FORWARD with one of ALL, FROM, IN,
+ * and a list of cursors
+ */
else if (Matches("FETCH|MOVE", "BACKWARD|FORWARD"))
- COMPLETE_WITH("ALL", "FROM", "IN");
+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors
+ " UNION SELECT 'ALL'"
+ " UNION SELECT 'FROM'"
+ " UNION SELECT 'IN'");
/*
* Complete FETCH <direction> with "FROM" or "IN". These are equivalent,
@@ -3185,7 +3236,13 @@ psql_completion(const char *text, int start, int end)
else if (Matches("FETCH|MOVE", "ABSOLUTE|BACKWARD|FORWARD|RELATIVE",
MatchAnyExcept("FROM|IN")) ||
Matches("FETCH|MOVE", "ALL|NEXT|PRIOR|FIRST|LAST"))
- COMPLETE_WITH("FROM", "IN");
+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors
+ " UNION SELECT 'FROM'"
+ " UNION SELECT 'IN'");
+ /* Complete FETCH <direction> "FROM" or "IN" with a list of cursors */
+ else if (HeadMatches("FETCH|MOVE") &&
+ TailMatches("FROM|IN"))
+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors);
/* FOREIGN DATA WRAPPER */
/* applies in ALTER/DROP FDW and in CREATE SERVER */
* RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion
2020-12-09 03:57 [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-04 11:56 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-05 06:02 ` RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-05 09:08 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@oss.nttdata.com>
2021-01-06 02:13 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-06 02:33 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@oss.nttdata.com>
2021-01-06 06:36 ` RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-07 01:01 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-07 01:59 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@oss.nttdata.com>
2021-01-07 03:42 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-07 04:30 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@oss.nttdata.com>
2021-01-07 06:53 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
@ 2021-01-07 08:28 ` Shinya11.Kato@nttdata.com
2021-01-07 12:32 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@oss.nttdata.com>
0 siblings, 1 reply; 22+ messages in thread
From: Shinya11.Kato@nttdata.com @ 2021-01-07 08:28 UTC (permalink / raw)
To: sawada.mshk@gmail.com; masao.fujii@oss.nttdata.com; +Cc: pgsql-hackers
>On Thu, Jan 7, 2021 at 1:30 PM Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote:
>>
>> On 2021/01/07 12:42, Masahiko Sawada wrote:
>> > On Thu, Jan 7, 2021 at 10:59 AM Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote:
>> >>
>> >> On 2021/01/07 10:01, Masahiko Sawada wrote:
>> >>> On Wed, Jan 6, 2021 at 3:37 PM <Shinya11(dot)Kato(at)nttdata(dot)com> wrote:
>> >>>>
>> >>>>> +#define Query_for_list_of_cursors \
>> >>>>> +" SELECT name FROM pg_cursors"\
>> >>>>>
>> >>>>> This query should be the following?
>> >>>>>
>> >>>>> " SELECT pg_catalog.quote_ident(name) "\
>> >>>>> " FROM pg_catalog.pg_cursors "\
>> >>>>> " WHERE substring(pg_catalog.quote_ident(name),1,%d)='%s'"
>> >>>>>
>> >>>>> +/* CLOSE */
>> >>>>> + else if (Matches("CLOSE"))
>> >>>>> + COMPLETE_WITH_QUERY(Query_for_list_of_cursors
>> >>>>> + " UNION ALL SELECT 'ALL'");
>> >>>>>
>> >>>>> "UNION ALL" should be "UNION"?
>> >>>>
>> >>>> Thank you for your advice, and I corrected them.
>> >>>>
>> >>>>>> + COMPLETE_WITH_QUERY(Query_for_list_of_cursors
>> >>>>>> + " UNION SELECT 'ABSOLUTE'"
>> >>>>>> + " UNION SELECT 'BACKWARD'"
>> >>>>>> + " UNION SELECT 'FORWARD'"
>> >>>>>> + " UNION SELECT 'RELATIVE'"
>> >>>>>> + " UNION SELECT 'ALL'"
>> >>>>>> + " UNION SELECT 'NEXT'"
>> >>>>>> + " UNION SELECT 'PRIOR'"
>> >>>>>> + " UNION SELECT 'FIRST'"
>> >>>>>> + " UNION SELECT 'LAST'"
>> >>>>>> + " UNION SELECT 'FROM'"
>> >>>>>> + " UNION SELECT 'IN'");
>> >>>>>>
>> >>>>>> This change makes psql unexpectedly output "FROM" and "IN" just after "FETCH".
>> >>>>>
>> >>>>> I think "FROM" and "IN" can be placed just after "FETCH". According to
>> >>>>> the documentation, the direction can be empty. For instance, we can do
>> >>>>> like:
>> >>>>
>> >>>> Thank you!
>> >>>>
>> >>>>> I've attached the patch improving the tab completion for DECLARE as an
>> >>>>> example. What do you think?
>> >>>>>
>> >>>>> BTW according to the documentation, the options of DECLARE statement
>> >>>>> (BINARY, INSENSITIVE, SCROLL, and NO SCROLL) are order-sensitive.
>> >>>>>
>> >>>>> DECLARE name [ BINARY ] [ INSENSITIVE ] [ [ NO ] SCROLL ]
>> >>>>> CURSOR [ { WITH | WITHOUT } HOLD ] FOR query
>> >>>>>
>> >>>>> But I realized that these options are actually order-insensitive. For
>> >>>>> instance, we can declare a cursor like:
>> >>>>>
>> >>>>> =# declare abc scroll binary cursor for select * from pg_class;
>> >>>>> DECLARE CURSOR
>> >>>>>
>> >>>>> The both parser code and documentation has been unchanged from 2003.
>> >>>>> Is it a documentation bug?
>> >>>>
>> >>>> Thank you for your patch, and it is good.
>> >>>> I cannot find the description "(BINARY, INSENSITIVE, SCROLL, and NO SCROLL) are order-sensitive."
>> >>>> I saw "The key words BINARY, INSENSITIVE, and SCROLL can appear in any order." , according to the documentation.
>> >>>
>> >>> Thanks, you're right. I missed that sentence. I still don't think the
>> >>> syntax of DECLARE statement in the documentation doesn't match its
>> >>> implementation but I agree that it's order-insensitive.
>> >>>
>> >>>> I made a new patch, but the amount of codes was large due to order-insensitive.
>> >>>
>> >>> Thank you for updating the patch!
>> >>>
>> >>> Yeah, I'm also afraid a bit that conditions will exponentially
>> >>> increase when a new option is added to DECLARE statement in the
>> >>> future. Looking at the parser code for DECLARE statement, we can put
>> >>> the same options multiple times (that's also why I don't think it
>> >>> matches). For instance,
>> >>>
>> >>> postgres(1:44758)=# begin;
>> >>> BEGIN
>> >>> postgres(1:44758)=# declare test binary binary binary cursor for
>> >>> select * from pg_class;
>> >>> DECLARE CURSOR
>> >>>
>> >>> So how about simplify the above code as follows?
>> >>>
>> >>> @@ -3005,8 +3014,23 @@ psql_completion(const char *text, int start, int end)
>> >>> else if (Matches("DECLARE", MatchAny))
>> >>> COMPLETE_WITH("BINARY", "INSENSITIVE", "SCROLL", "NO SCROLL",
>> >>> "CURSOR");
>> >>> + /*
>> >>> + * Complete DECLARE <name> with one of BINARY, INSENSITIVE, SCROLL,
>> >>> + * NO SCROLL, and CURSOR. The tail doesn't match any keywords for
>> >>> + * DECLARE, assume we want options.
>> >>> + */
>> >>> + else if (HeadMatches("DECLARE", MatchAny, "*") &&
>> >>> + TailMatches(MatchAnyExcept("CURSOR|WITH|WITHOUT|HOLD|FOR")))
>> >>> + COMPLETE_WITH("BINARY", "INSENSITIVE", "SCROLL", "NO SCROLL",
>> >>> + "CURSOR");
>> >>
>> >> This change seems to cause "DECLARE ... CURSOR FOR SELECT <tab>" to
>> >> unexpectedly output BINARY, INSENSITIVE, etc.
>> >
>> > Indeed. Is the following not complete but much better?
>>
>> Yes, I think that's better.
>>
>> >
>> > @@ -3002,11 +3011,18 @@ psql_completion(const char *text, int start, int end)
>> > " UNION SELECT 'ALL'");
>> >
>> > /* DECLARE */
>> > - else if (Matches("DECLARE", MatchAny))
>> > - COMPLETE_WITH("BINARY", "INSENSITIVE", "SCROLL", "NO SCROLL",
>> > - "CURSOR");
>> > + /*
>> > + * Complete DECLARE <name> with one of BINARY, INSENSITIVE, SCROLL,
>> > + * NO SCROLL, and CURSOR. If the tail is any one of options, assume we
>> > + * still want options.
>> > + */
>> > + else if (Matches("DECLARE", MatchAny) ||
>> > + TailMatches("BINARY|INSENSITIVE|SCROLL|NO"))
>> > + COMPLETE_WITH("BINARY", "INSENSITIVE", "SCROLL", "NO SCROLL", "CURSOR");
>>
>> This change seems to cause "DECLARE ... NO <tab>" to unexpectedly output
>> "NO SCROLL". Also this change seems to cause "COPY ... (FORMAT BINARY <tab>"
>> to unexpectedly output BINARY, CURSOR, etc.
>
>Oops, I missed the HeadMatches(). Thank you for pointing this out.
>
>I've attached the updated patch including Kato-san's changes. I
>think the tab completion support for DECLARE added by this patch
>works better.
Thank you very much for the new patch!
I checked the operation and I think it is good.
Regards,
Shinya Kato
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion
2020-12-09 03:57 [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-04 11:56 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-05 06:02 ` RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-05 09:08 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@oss.nttdata.com>
2021-01-06 02:13 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-06 02:33 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@oss.nttdata.com>
2021-01-06 06:36 ` RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-07 01:01 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-07 01:59 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@oss.nttdata.com>
2021-01-07 03:42 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-07 04:30 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@oss.nttdata.com>
2021-01-07 06:53 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-07 08:28 ` RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
@ 2021-01-07 12:32 ` Fujii Masao <masao.fujii@oss.nttdata.com>
2021-01-11 05:39 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
0 siblings, 1 reply; 22+ messages in thread
From: Fujii Masao @ 2021-01-07 12:32 UTC (permalink / raw)
To: Shinya11.Kato@nttdata.com; sawada.mshk@gmail.com; +Cc: pgsql-hackers
On 2021/01/07 17:28, Shinya11.Kato@nttdata.com wrote:
>> On Thu, Jan 7, 2021 at 1:30 PM Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote:
>>>
>>> On 2021/01/07 12:42, Masahiko Sawada wrote:
>>>> On Thu, Jan 7, 2021 at 10:59 AM Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote:
>>>>>
>>>>> On 2021/01/07 10:01, Masahiko Sawada wrote:
>>>>>> On Wed, Jan 6, 2021 at 3:37 PM <Shinya11(dot)Kato(at)nttdata(dot)com> wrote:
>>>>>>>
>>>>>>>> +#define Query_for_list_of_cursors \
>>>>>>>> +" SELECT name FROM pg_cursors"\
>>>>>>>>
>>>>>>>> This query should be the following?
>>>>>>>>
>>>>>>>> " SELECT pg_catalog.quote_ident(name) "\
>>>>>>>> " FROM pg_catalog.pg_cursors "\
>>>>>>>> " WHERE substring(pg_catalog.quote_ident(name),1,%d)='%s'"
>>>>>>>>
>>>>>>>> +/* CLOSE */
>>>>>>>> + else if (Matches("CLOSE"))
>>>>>>>> + COMPLETE_WITH_QUERY(Query_for_list_of_cursors
>>>>>>>> + " UNION ALL SELECT 'ALL'");
>>>>>>>>
>>>>>>>> "UNION ALL" should be "UNION"?
>>>>>>>
>>>>>>> Thank you for your advice, and I corrected them.
>>>>>>>
>>>>>>>>> + COMPLETE_WITH_QUERY(Query_for_list_of_cursors
>>>>>>>>> + " UNION SELECT 'ABSOLUTE'"
>>>>>>>>> + " UNION SELECT 'BACKWARD'"
>>>>>>>>> + " UNION SELECT 'FORWARD'"
>>>>>>>>> + " UNION SELECT 'RELATIVE'"
>>>>>>>>> + " UNION SELECT 'ALL'"
>>>>>>>>> + " UNION SELECT 'NEXT'"
>>>>>>>>> + " UNION SELECT 'PRIOR'"
>>>>>>>>> + " UNION SELECT 'FIRST'"
>>>>>>>>> + " UNION SELECT 'LAST'"
>>>>>>>>> + " UNION SELECT 'FROM'"
>>>>>>>>> + " UNION SELECT 'IN'");
>>>>>>>>>
>>>>>>>>> This change makes psql unexpectedly output "FROM" and "IN" just after "FETCH".
>>>>>>>>
>>>>>>>> I think "FROM" and "IN" can be placed just after "FETCH". According to
>>>>>>>> the documentation, the direction can be empty. For instance, we can do
>>>>>>>> like:
>>>>>>>
>>>>>>> Thank you!
>>>>>>>
>>>>>>>> I've attached the patch improving the tab completion for DECLARE as an
>>>>>>>> example. What do you think?
>>>>>>>>
>>>>>>>> BTW according to the documentation, the options of DECLARE statement
>>>>>>>> (BINARY, INSENSITIVE, SCROLL, and NO SCROLL) are order-sensitive.
>>>>>>>>
>>>>>>>> DECLARE name [ BINARY ] [ INSENSITIVE ] [ [ NO ] SCROLL ]
>>>>>>>> CURSOR [ { WITH | WITHOUT } HOLD ] FOR query
>>>>>>>>
>>>>>>>> But I realized that these options are actually order-insensitive. For
>>>>>>>> instance, we can declare a cursor like:
>>>>>>>>
>>>>>>>> =# declare abc scroll binary cursor for select * from pg_class;
>>>>>>>> DECLARE CURSOR
>>>>>>>>
>>>>>>>> The both parser code and documentation has been unchanged from 2003.
>>>>>>>> Is it a documentation bug?
>>>>>>>
>>>>>>> Thank you for your patch, and it is good.
>>>>>>> I cannot find the description "(BINARY, INSENSITIVE, SCROLL, and NO SCROLL) are order-sensitive."
>>>>>>> I saw "The key words BINARY, INSENSITIVE, and SCROLL can appear in any order." , according to the documentation.
>>>>>>
>>>>>> Thanks, you're right. I missed that sentence. I still don't think the
>>>>>> syntax of DECLARE statement in the documentation doesn't match its
>>>>>> implementation but I agree that it's order-insensitive.
>>>>>>
>>>>>>> I made a new patch, but the amount of codes was large due to order-insensitive.
>>>>>>
>>>>>> Thank you for updating the patch!
>>>>>>
>>>>>> Yeah, I'm also afraid a bit that conditions will exponentially
>>>>>> increase when a new option is added to DECLARE statement in the
>>>>>> future. Looking at the parser code for DECLARE statement, we can put
>>>>>> the same options multiple times (that's also why I don't think it
>>>>>> matches). For instance,
>>>>>>
>>>>>> postgres(1:44758)=# begin;
>>>>>> BEGIN
>>>>>> postgres(1:44758)=# declare test binary binary binary cursor for
>>>>>> select * from pg_class;
>>>>>> DECLARE CURSOR
>>>>>>
>>>>>> So how about simplify the above code as follows?
>>>>>>
>>>>>> @@ -3005,8 +3014,23 @@ psql_completion(const char *text, int start, int end)
>>>>>> else if (Matches("DECLARE", MatchAny))
>>>>>> COMPLETE_WITH("BINARY", "INSENSITIVE", "SCROLL", "NO SCROLL",
>>>>>> "CURSOR");
>>>>>> + /*
>>>>>> + * Complete DECLARE <name> with one of BINARY, INSENSITIVE, SCROLL,
>>>>>> + * NO SCROLL, and CURSOR. The tail doesn't match any keywords for
>>>>>> + * DECLARE, assume we want options.
>>>>>> + */
>>>>>> + else if (HeadMatches("DECLARE", MatchAny, "*") &&
>>>>>> + TailMatches(MatchAnyExcept("CURSOR|WITH|WITHOUT|HOLD|FOR")))
>>>>>> + COMPLETE_WITH("BINARY", "INSENSITIVE", "SCROLL", "NO SCROLL",
>>>>>> + "CURSOR");
>>>>>
>>>>> This change seems to cause "DECLARE ... CURSOR FOR SELECT <tab>" to
>>>>> unexpectedly output BINARY, INSENSITIVE, etc.
>>>>
>>>> Indeed. Is the following not complete but much better?
>>>
>>> Yes, I think that's better.
>>>
>>>>
>>>> @@ -3002,11 +3011,18 @@ psql_completion(const char *text, int start, int end)
>>>> " UNION SELECT 'ALL'");
>>>>
>>>> /* DECLARE */
>>>> - else if (Matches("DECLARE", MatchAny))
>>>> - COMPLETE_WITH("BINARY", "INSENSITIVE", "SCROLL", "NO SCROLL",
>>>> - "CURSOR");
>>>> + /*
>>>> + * Complete DECLARE <name> with one of BINARY, INSENSITIVE, SCROLL,
>>>> + * NO SCROLL, and CURSOR. If the tail is any one of options, assume we
>>>> + * still want options.
>>>> + */
>>>> + else if (Matches("DECLARE", MatchAny) ||
>>>> + TailMatches("BINARY|INSENSITIVE|SCROLL|NO"))
>>>> + COMPLETE_WITH("BINARY", "INSENSITIVE", "SCROLL", "NO SCROLL", "CURSOR");
>>>
>>> This change seems to cause "DECLARE ... NO <tab>" to unexpectedly output
>>> "NO SCROLL". Also this change seems to cause "COPY ... (FORMAT BINARY <tab>"
>>> to unexpectedly output BINARY, CURSOR, etc.
>>
>> Oops, I missed the HeadMatches(). Thank you for pointing this out.
>>
>> I've attached the updated patch including Kato-san's changes. I
>> think the tab completion support for DECLARE added by this patch
>> works better.
Thanks!
+ /* Complete with more options */
+ else if (HeadMatches("DECLARE", MatchAny, "BINARY|INSENSITIVE|SCROLL|NO") &&
+ TailMatches("BINARY|INSENSITIVE|SCROLL|NO"))
Seems "MatchAny, "BINARY|INSENSITIVE|SCROLL|NO"" is not necessary. Right?
If this is true, I'd like to refactor the code a bit.
What about the attached patch?
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
Attachments:
fix_tab_complete_close_fetch_move_v5.patch (text/plain, 4.5K) download
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 9dcab0d2fa..aa994a3af0 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -976,6 +976,11 @@ static const SchemaQuery Query_for_list_of_statistics = {
" and pg_catalog.pg_table_is_visible(c2.oid)"\
" and c2.relispartition = 'true'"
+#define Query_for_list_of_cursors \
+" SELECT pg_catalog.quote_ident(name) "\
+" FROM pg_catalog.pg_cursors "\
+" WHERE substring(pg_catalog.quote_ident(name),1,%d)='%s'"
+
/*
* These object types were introduced later than our support cutoff of
* server version 7.4. We use the VersionedQuery infrastructure so that
@@ -2284,6 +2289,10 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH_VERSIONED_SCHEMA_QUERY(Query_for_list_of_procedures, NULL);
else if (Matches("CALL", MatchAny))
COMPLETE_WITH("(");
+/* CLOSE */
+ else if (Matches("CLOSE"))
+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors
+ " UNION SELECT 'ALL'");
/* CLUSTER */
else if (Matches("CLUSTER"))
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_clusterables, "UNION SELECT 'VERBOSE'");
@@ -3002,11 +3011,39 @@ psql_completion(const char *text, int start, int end)
" UNION SELECT 'ALL'");
/* DECLARE */
+
+ /*
+ * Complete DECLARE <name> with one of BINARY, INSENSITIVE, SCROLL, NO
+ * SCROLL, and CURSOR.
+ */
else if (Matches("DECLARE", MatchAny))
COMPLETE_WITH("BINARY", "INSENSITIVE", "SCROLL", "NO SCROLL",
"CURSOR");
+
+ /*
+ * Complete DECLARE <name> [options] with one of BINARY, INSENSITIVE,
+ * SCROLL, NO SCROLL, and CURSOR. If the tail is any one of options,
+ * assume we still want options.
+ */
+ else if (HeadMatches("DECLARE") && TailMatches("BINARY|INSENSITIVE|SCROLL"))
+ COMPLETE_WITH("BINARY", "INSENSITIVE", "SCROLL", "NO SCROLL",
+ "CURSOR");
+ /* Complete DECLARE <name> [options] NO with SCROLL */
+ else if (HeadMatches("DECLARE") && TailMatches("NO"))
+ COMPLETE_WITH("SCROLL");
+
+ /*
+ * Complete DECLARE ... CURSOR with one of WITH HOLD, WITHOUT HOLD, and
+ * FOR
+ */
else if (HeadMatches("DECLARE") && TailMatches("CURSOR"))
COMPLETE_WITH("WITH HOLD", "WITHOUT HOLD", "FOR");
+ /* Complete DECLARE ... CURSOR WITH|WITHOUT with HOLD */
+ else if (HeadMatches("DECLARE") && TailMatches("CURSOR", "WITH|WITHOUT"))
+ COMPLETE_WITH("HOLD");
+ /* Complete DECLARE ... CURSOR WITH|WITHOUT HOLD with FOR */
+ else if (HeadMatches("DECLARE") && TailMatches("CURSOR", "WITH|WITHOUT", "HOLD"))
+ COMPLETE_WITH("FOR");
/* DELETE --- can be inside EXPLAIN, RULE, etc */
/* ... despite which, only complete DELETE with FROM at start of line */
@@ -3167,15 +3204,31 @@ psql_completion(const char *text, int start, int end)
/*
* Complete FETCH with one of ABSOLUTE, BACKWARD, FORWARD, RELATIVE, ALL,
- * NEXT, PRIOR, FIRST, LAST
+ * NEXT, PRIOR, FIRST, LAST, FROM, IN, and a list of cursors
*/
else if (Matches("FETCH|MOVE"))
- COMPLETE_WITH("ABSOLUTE", "BACKWARD", "FORWARD", "RELATIVE",
- "ALL", "NEXT", "PRIOR", "FIRST", "LAST");
+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors
+ " UNION SELECT 'ABSOLUTE'"
+ " UNION SELECT 'BACKWARD'"
+ " UNION SELECT 'FORWARD'"
+ " UNION SELECT 'RELATIVE'"
+ " UNION SELECT 'ALL'"
+ " UNION SELECT 'NEXT'"
+ " UNION SELECT 'PRIOR'"
+ " UNION SELECT 'FIRST'"
+ " UNION SELECT 'LAST'"
+ " UNION SELECT 'FROM'"
+ " UNION SELECT 'IN'");
- /* Complete FETCH BACKWARD or FORWARD with one of ALL, FROM, IN */
+ /*
+ * Complete FETCH BACKWARD or FORWARD with one of ALL, FROM, IN, and a
+ * list of cursors
+ */
else if (Matches("FETCH|MOVE", "BACKWARD|FORWARD"))
- COMPLETE_WITH("ALL", "FROM", "IN");
+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors
+ " UNION SELECT 'ALL'"
+ " UNION SELECT 'FROM'"
+ " UNION SELECT 'IN'");
/*
* Complete FETCH <direction> with "FROM" or "IN". These are equivalent,
@@ -3185,7 +3238,13 @@ psql_completion(const char *text, int start, int end)
else if (Matches("FETCH|MOVE", "ABSOLUTE|BACKWARD|FORWARD|RELATIVE",
MatchAnyExcept("FROM|IN")) ||
Matches("FETCH|MOVE", "ALL|NEXT|PRIOR|FIRST|LAST"))
- COMPLETE_WITH("FROM", "IN");
+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors
+ " UNION SELECT 'FROM'"
+ " UNION SELECT 'IN'");
+ /* Complete FETCH <direction> "FROM" or "IN" with a list of cursors */
+ else if (HeadMatches("FETCH|MOVE") &&
+ TailMatches("FROM|IN"))
+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors);
/* FOREIGN DATA WRAPPER */
/* applies in ALTER/DROP FDW and in CREATE SERVER */
* Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion
2020-12-09 03:57 [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-04 11:56 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-05 06:02 ` RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-05 09:08 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@oss.nttdata.com>
2021-01-06 02:13 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-06 02:33 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@oss.nttdata.com>
2021-01-06 06:36 ` RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-07 01:01 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-07 01:59 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@oss.nttdata.com>
2021-01-07 03:42 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-07 04:30 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@oss.nttdata.com>
2021-01-07 06:53 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-07 08:28 ` RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-07 12:32 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@oss.nttdata.com>
@ 2021-01-11 05:39 ` Masahiko Sawada <sawada.mshk@gmail.com>
0 siblings, 0 replies; 22+ messages in thread
From: Masahiko Sawada @ 2021-01-11 05:39 UTC (permalink / raw)
To: Fujii Masao <masao.fujii@oss.nttdata.com>; +Cc: pgsql-hackers
On Thu, Jan 7, 2021 at 9:32 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>
>
>
> On 2021/01/07 17:28, Shinya11.Kato@nttdata.com wrote:
> >> On Thu, Jan 7, 2021 at 1:30 PM Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote:
> >>>
> >>> On 2021/01/07 12:42, Masahiko Sawada wrote:
> >>>> On Thu, Jan 7, 2021 at 10:59 AM Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote:
> >>>>>
> >>>>> On 2021/01/07 10:01, Masahiko Sawada wrote:
> >>>>>> On Wed, Jan 6, 2021 at 3:37 PM <Shinya11(dot)Kato(at)nttdata(dot)com> wrote:
> >>>>>>>
> >>>>>>>> +#define Query_for_list_of_cursors \
> >>>>>>>> +" SELECT name FROM pg_cursors"\
> >>>>>>>>
> >>>>>>>> This query should be the following?
> >>>>>>>>
> >>>>>>>> " SELECT pg_catalog.quote_ident(name) "\
> >>>>>>>> " FROM pg_catalog.pg_cursors "\
> >>>>>>>> " WHERE substring(pg_catalog.quote_ident(name),1,%d)='%s'"
> >>>>>>>>
> >>>>>>>> +/* CLOSE */
> >>>>>>>> + else if (Matches("CLOSE"))
> >>>>>>>> + COMPLETE_WITH_QUERY(Query_for_list_of_cursors
> >>>>>>>> + " UNION ALL SELECT 'ALL'");
> >>>>>>>>
> >>>>>>>> "UNION ALL" should be "UNION"?
> >>>>>>>
> >>>>>>> Thank you for your advice, and I corrected them.
> >>>>>>>
> >>>>>>>>> + COMPLETE_WITH_QUERY(Query_for_list_of_cursors
> >>>>>>>>> + " UNION SELECT 'ABSOLUTE'"
> >>>>>>>>> + " UNION SELECT 'BACKWARD'"
> >>>>>>>>> + " UNION SELECT 'FORWARD'"
> >>>>>>>>> + " UNION SELECT 'RELATIVE'"
> >>>>>>>>> + " UNION SELECT 'ALL'"
> >>>>>>>>> + " UNION SELECT 'NEXT'"
> >>>>>>>>> + " UNION SELECT 'PRIOR'"
> >>>>>>>>> + " UNION SELECT 'FIRST'"
> >>>>>>>>> + " UNION SELECT 'LAST'"
> >>>>>>>>> + " UNION SELECT 'FROM'"
> >>>>>>>>> + " UNION SELECT 'IN'");
> >>>>>>>>>
> >>>>>>>>> This change makes psql unexpectedly output "FROM" and "IN" just after "FETCH".
> >>>>>>>>
> >>>>>>>> I think "FROM" and "IN" can be placed just after "FETCH". According to
> >>>>>>>> the documentation, the direction can be empty. For instance, we can do
> >>>>>>>> like:
> >>>>>>>
> >>>>>>> Thank you!
> >>>>>>>
> >>>>>>>> I've attached the patch improving the tab completion for DECLARE as an
> >>>>>>>> example. What do you think?
> >>>>>>>>
> >>>>>>>> BTW according to the documentation, the options of DECLARE statement
> >>>>>>>> (BINARY, INSENSITIVE, SCROLL, and NO SCROLL) are order-sensitive.
> >>>>>>>>
> >>>>>>>> DECLARE name [ BINARY ] [ INSENSITIVE ] [ [ NO ] SCROLL ]
> >>>>>>>> CURSOR [ { WITH | WITHOUT } HOLD ] FOR query
> >>>>>>>>
> >>>>>>>> But I realized that these options are actually order-insensitive. For
> >>>>>>>> instance, we can declare a cursor like:
> >>>>>>>>
> >>>>>>>> =# declare abc scroll binary cursor for select * from pg_class;
> >>>>>>>> DECLARE CURSOR
> >>>>>>>>
> >>>>>>>> The both parser code and documentation has been unchanged from 2003.
> >>>>>>>> Is it a documentation bug?
> >>>>>>>
> >>>>>>> Thank you for your patch, and it is good.
> >>>>>>> I cannot find the description "(BINARY, INSENSITIVE, SCROLL, and NO SCROLL) are order-sensitive."
> >>>>>>> I saw "The key words BINARY, INSENSITIVE, and SCROLL can appear in any order." , according to the documentation.
> >>>>>>
> >>>>>> Thanks, you're right. I missed that sentence. I still don't think the
> >>>>>> syntax of DECLARE statement in the documentation doesn't match its
> >>>>>> implementation but I agree that it's order-insensitive.
> >>>>>>
> >>>>>>> I made a new patch, but the amount of codes was large due to order-insensitive.
> >>>>>>
> >>>>>> Thank you for updating the patch!
> >>>>>>
> >>>>>> Yeah, I'm also afraid a bit that conditions will exponentially
> >>>>>> increase when a new option is added to DECLARE statement in the
> >>>>>> future. Looking at the parser code for DECLARE statement, we can put
> >>>>>> the same options multiple times (that's also why I don't think it
> >>>>>> matches). For instance,
> >>>>>>
> >>>>>> postgres(1:44758)=# begin;
> >>>>>> BEGIN
> >>>>>> postgres(1:44758)=# declare test binary binary binary cursor for
> >>>>>> select * from pg_class;
> >>>>>> DECLARE CURSOR
> >>>>>>
> >>>>>> So how about simplify the above code as follows?
> >>>>>>
> >>>>>> @@ -3005,8 +3014,23 @@ psql_completion(const char *text, int start, int end)
> >>>>>> else if (Matches("DECLARE", MatchAny))
> >>>>>> COMPLETE_WITH("BINARY", "INSENSITIVE", "SCROLL", "NO SCROLL",
> >>>>>> "CURSOR");
> >>>>>> + /*
> >>>>>> + * Complete DECLARE <name> with one of BINARY, INSENSITIVE, SCROLL,
> >>>>>> + * NO SCROLL, and CURSOR. The tail doesn't match any keywords for
> >>>>>> + * DECLARE, assume we want options.
> >>>>>> + */
> >>>>>> + else if (HeadMatches("DECLARE", MatchAny, "*") &&
> >>>>>> + TailMatches(MatchAnyExcept("CURSOR|WITH|WITHOUT|HOLD|FOR")))
> >>>>>> + COMPLETE_WITH("BINARY", "INSENSITIVE", "SCROLL", "NO SCROLL",
> >>>>>> + "CURSOR");
> >>>>>
> >>>>> This change seems to cause "DECLARE ... CURSOR FOR SELECT <tab>" to
> >>>>> unexpectedly output BINARY, INSENSITIVE, etc.
> >>>>
> >>>> Indeed. Is the following not complete but much better?
> >>>
> >>> Yes, I think that's better.
> >>>
> >>>>
> >>>> @@ -3002,11 +3011,18 @@ psql_completion(const char *text, int start, int end)
> >>>> " UNION SELECT 'ALL'");
> >>>>
> >>>> /* DECLARE */
> >>>> - else if (Matches("DECLARE", MatchAny))
> >>>> - COMPLETE_WITH("BINARY", "INSENSITIVE", "SCROLL", "NO SCROLL",
> >>>> - "CURSOR");
> >>>> + /*
> >>>> + * Complete DECLARE <name> with one of BINARY, INSENSITIVE, SCROLL,
> >>>> + * NO SCROLL, and CURSOR. If the tail is any one of options, assume we
> >>>> + * still want options.
> >>>> + */
> >>>> + else if (Matches("DECLARE", MatchAny) ||
> >>>> + TailMatches("BINARY|INSENSITIVE|SCROLL|NO"))
> >>>> + COMPLETE_WITH("BINARY", "INSENSITIVE", "SCROLL", "NO SCROLL", "CURSOR");
> >>>
> >>> This change seems to cause "DECLARE ... NO <tab>" to unexpectedly output
> >>> "NO SCROLL". Also this change seems to cause "COPY ... (FORMAT BINARY <tab>"
> >>> to unexpectedly output BINARY, CURSOR, etc.
> >>
> >> Oops, I missed the HeadMatches(). Thank you for pointing this out.
> >>
> >> I've attached the updated patch including Kato-san's changes. I
> >> think the tab completion support for DECLARE added by this patch
> >> works better.
>
> Thanks!
>
> + /* Complete with more options */
> + else if (HeadMatches("DECLARE", MatchAny, "BINARY|INSENSITIVE|SCROLL|NO") &&
> + TailMatches("BINARY|INSENSITIVE|SCROLL|NO"))
>
> Seems "MatchAny, "BINARY|INSENSITIVE|SCROLL|NO"" is not necessary. Right?
>
Right.
> If this is true, I'd like to refactor the code a bit.
> What about the attached patch?
Thank you for updating the patch! Looks good to me.
Regards,
--
Masahiko Sawada
EnterpriseDB: https://www.enterprisedb.com/
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion
2020-12-09 03:57 [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-04 11:56 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-05 06:02 ` RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
@ 2021-01-05 09:56 ` Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-11 14:00 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Peter Eisentraut <peter.eisentraut@enterprisedb.com>
1 sibling, 1 reply; 22+ messages in thread
From: Masahiko Sawada @ 2021-01-05 09:56 UTC (permalink / raw)
To: Shinya11.Kato@nttdata.com; +Cc: pgsql-hackers
On Tue, Jan 5, 2021 at 3:03 PM <Shinya11.Kato@nttdata.com> wrote:
>
> Thank you for your review!
> I fixed some codes and attach a new patch.
Thank you for updating the patch!
>
> >When I applied the patch, I got the following whitespace warnings:
> >
> >$ git apply ~/patches/fix_tab_complete_close_fetch_move.patch
> >/home/masahiko/patches/fix_tab_complete_close_fetch_move.patch:40:
> >indent with spaces.
> > COMPLETE_WITH_QUERY(Query_for_list_of_cursors
> >/home/masahiko/patches/fix_tab_complete_close_fetch_move.patch:41:
> >indent with spaces.
> > " UNION SELECT 'ABSOLUTE'"
> >/home/masahiko/patches/fix_tab_complete_close_fetch_move.patch:42:
> >indent with spaces.
> > " UNION SELECT 'BACKWARD'"
> >/home/masahiko/patches/fix_tab_complete_close_fetch_move.patch:43:
> >indent with spaces.
> > " UNION SELECT 'FORWARD'"
> >/home/masahiko/patches/fix_tab_complete_close_fetch_move.patch:44:
> >indent with spaces.
> > " UNION SELECT 'RELATIVE'"
> >warning: squelched 19 whitespace errors
> >warning: 24 lines add whitespace errors.
> >
> >I recommend you checking whitespaces or running pgindent.
>
> Thank you for your advice and I have corrected it.
>
> >Here are some comments:
> >
> > /*
> >- * Complete FETCH with one of ABSOLUTE, BACKWARD, FORWARD,
> >RELATIVE, ALL,
> >- * NEXT, PRIOR, FIRST, LAST
> >+ * Complete FETCH with a list of cursors and one of ABSOLUTE,
> >BACKWARD, FORWARD, RELATIVE, ALL,
> >+ * NEXT, PRIOR, FIRST, LAST, FROM, IN
> > */
> >
> >Maybe I think the commend should say:
> >
> >+ * Complete FETCH with one of ABSOLUTE, BACKWARD, FORWARD,
> >RELATIVE, ALL,
> >+ * NEXT, PRIOR, FIRST, LAST, FROM, IN, and a list of cursors
> >
> >Other updates of the comment seem to have the same issue.
> >
> >Or I think we can omit the details from the comment since it seems redundant
> >information. We can read it from the arguments of the following
> >COMPLETE_WITH_QUERY().
>
> It certainly seems redundant, so I deleted them.
>
> >---
> >- /*
> >- * Complete FETCH <direction> with "FROM" or "IN". These are equivalent,
> >- * but we may as well tab-complete both: perhaps some users prefer one
> >- * variant or the other.
> >- */
> >+ /* Complete FETCH <direction> with a list of cursors and "FROM" or
> >+ "IN" */
> >
> >Why did you remove the second sentence in the above comment?
>
> I had misunderstood the meaning and deleted it.
> I deleted it as well as above, but would you prefer it to be there?
I would leave it. I realized this area is recently updated by commit
8176afd8b7. In that change, the comments were updated rather than
removed. So it might be better to leave them. Sorry for confusing you.
>
> >---
> >The patch improves tab completion for CLOSE, FETCH, and MOVE but is there
> >any reason why you didn't do that for DECLARE? I think DECLARE also can be
> >improved and it's a good timing for that.
>
> I wanted to improve tab completion for DECLARE, but I couldn't find anything to improve.
> Please let me know if there are any codes that can be improved.
I've attached the patch improving the tab completion for DECLARE as an
example. What do you think?
BTW according to the documentation, the options of DECLARE statement
(BINARY, INSENSITIVE, SCROLL, and NO SCROLL) are order-sensitive.
DECLARE name [ BINARY ] [ INSENSITIVE ] [ [ NO ] SCROLL ]
CURSOR [ { WITH | WITHOUT } HOLD ] FOR query
But I realized that these options are actually order-insensitive. For
instance, we can declare a cursor like:
=# declare abc scroll binary cursor for select * from pg_class;
DECLARE CURSOR
The both parser code and documentation has been unchanged from 2003.
Is it a documentation bug?
Regards,
--
Masahiko Sawada
EnterpriseDB: https://www.enterprisedb.com/
Attachments:
tab_completion_declare_stmt.patch (application/x-patch, 1.0K) download
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index b2671469aa..eb937bf156 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -3012,8 +3012,17 @@ psql_completion(const char *text, int start, int end)
else if (Matches("DECLARE", MatchAny))
COMPLETE_WITH("BINARY", "INSENSITIVE", "SCROLL", "NO SCROLL",
"CURSOR");
+ else if (Matches("DECLARE", MatchAny, "BINARY"))
+ COMPLETE_WITH("INSENSITIVE", "SCROLL", "NO SCROLL", "CURSOR");
+ else if (Matches("DECLARE", MatchAny, "BINARY", "INSENSITIVE"))
+ COMPLETE_WITH("SCROLL", "NO SCROLL", "CURSOR");
+ else if (HeadMatches("DECLARE", MatchAny, "BINARY", "INSENSITIVE") &&
+ TailMatches("SCROLL"))
+ COMPLETE_WITH("CURSOR");
else if (HeadMatches("DECLARE") && TailMatches("CURSOR"))
COMPLETE_WITH("WITH HOLD", "WITHOUT HOLD", "FOR");
+ else if (HeadMatches("DECLARE") && TailMatches("HOLD"))
+ COMPLETE_WITH("FOR");
/* DELETE --- can be inside EXPLAIN, RULE, etc */
/* ... despite which, only complete DELETE with FROM at start of line */
* Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion
2020-12-09 03:57 [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-04 11:56 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-05 06:02 ` RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-05 09:56 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
@ 2021-01-11 14:00 ` Peter Eisentraut <peter.eisentraut@enterprisedb.com>
2021-01-12 00:59 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
0 siblings, 1 reply; 22+ messages in thread
From: Peter Eisentraut @ 2021-01-11 14:00 UTC (permalink / raw)
To: Masahiko Sawada <sawada.mshk@gmail.com>; Shinya11.Kato@nttdata.com; +Cc: pgsql-hackers
On 2021-01-05 10:56, Masahiko Sawada wrote:
> BTW according to the documentation, the options of DECLARE statement
> (BINARY, INSENSITIVE, SCROLL, and NO SCROLL) are order-sensitive.
>
> DECLARE name [ BINARY ] [ INSENSITIVE ] [ [ NO ] SCROLL ]
> CURSOR [ { WITH | WITHOUT } HOLD ] FOR query
>
> But I realized that these options are actually order-insensitive. For
> instance, we can declare a cursor like:
>
> =# declare abc scroll binary cursor for select * from pg_class;
> DECLARE CURSOR
>
> The both parser code and documentation has been unchanged from 2003.
> Is it a documentation bug?
According to the SQL standard, the ordering of the cursor properties is
fixed. Even if the PostgreSQL parser offers more flexibility, I think
we should continue to encourage writing the clauses in the standard order.
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion
2020-12-09 03:57 [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-04 11:56 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-05 06:02 ` RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-05 09:56 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-11 14:00 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Peter Eisentraut <peter.eisentraut@enterprisedb.com>
@ 2021-01-12 00:59 ` Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-12 02:09 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@gmail.com>
0 siblings, 1 reply; 22+ messages in thread
From: Masahiko Sawada @ 2021-01-12 00:59 UTC (permalink / raw)
To: Peter Eisentraut <peter.eisentraut@enterprisedb.com>; +Cc: pgsql-hackers
On Mon, Jan 11, 2021 at 11:00 PM Peter Eisentraut
<peter.eisentraut@enterprisedb.com> wrote:
>
> On 2021-01-05 10:56, Masahiko Sawada wrote:
> > BTW according to the documentation, the options of DECLARE statement
> > (BINARY, INSENSITIVE, SCROLL, and NO SCROLL) are order-sensitive.
> >
> > DECLARE name [ BINARY ] [ INSENSITIVE ] [ [ NO ] SCROLL ]
> > CURSOR [ { WITH | WITHOUT } HOLD ] FOR query
> >
> > But I realized that these options are actually order-insensitive. For
> > instance, we can declare a cursor like:
> >
> > =# declare abc scroll binary cursor for select * from pg_class;
> > DECLARE CURSOR
> >
> > The both parser code and documentation has been unchanged from 2003.
> > Is it a documentation bug?
>
> According to the SQL standard, the ordering of the cursor properties is
> fixed. Even if the PostgreSQL parser offers more flexibility, I think
> we should continue to encourage writing the clauses in the standard order.
Thanks for your comment. Agreed.
So regarding the tab completion for DECLARE statement, perhaps it
would be better to follow the documentation? In the current proposed
patch, we complete it with the options in any order.
Regards,
--
Masahiko Sawada
EnterpriseDB: https://www.enterprisedb.com/
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion
2020-12-09 03:57 [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-04 11:56 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-05 06:02 ` RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-05 09:56 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-11 14:00 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Peter Eisentraut <peter.eisentraut@enterprisedb.com>
2021-01-12 00:59 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
@ 2021-01-12 02:09 ` Fujii Masao <masao.fujii@gmail.com>
2021-01-13 04:55 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@gmail.com>
0 siblings, 1 reply; 22+ messages in thread
From: Fujii Masao @ 2021-01-12 02:09 UTC (permalink / raw)
To: Masahiko Sawada <sawada.mshk@gmail.com>; +Cc: Peter Eisentraut <peter.eisentraut@enterprisedb.com>; pgsql-hackers
On Tue, Jan 12, 2021 at 10:00 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Mon, Jan 11, 2021 at 11:00 PM Peter Eisentraut
> <peter.eisentraut@enterprisedb.com> wrote:
> >
> > On 2021-01-05 10:56, Masahiko Sawada wrote:
> > > BTW according to the documentation, the options of DECLARE statement
> > > (BINARY, INSENSITIVE, SCROLL, and NO SCROLL) are order-sensitive.
> > >
> > > DECLARE name [ BINARY ] [ INSENSITIVE ] [ [ NO ] SCROLL ]
> > > CURSOR [ { WITH | WITHOUT } HOLD ] FOR query
> > >
> > > But I realized that these options are actually order-insensitive. For
> > > instance, we can declare a cursor like:
> > >
> > > =# declare abc scroll binary cursor for select * from pg_class;
> > > DECLARE CURSOR
> > >
> > > The both parser code and documentation has been unchanged from 2003.
> > > Is it a documentation bug?
> >
> > According to the SQL standard, the ordering of the cursor properties is
> > fixed. Even if the PostgreSQL parser offers more flexibility, I think
> > we should continue to encourage writing the clauses in the standard order.
>
> Thanks for your comment. Agreed.
>
> So regarding the tab completion for DECLARE statement, perhaps it
> would be better to follow the documentation?
IMO yes because it's less confusing to make the document and
tab-completion consistent.
Regards,
--
Fujii Masao
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion
2020-12-09 03:57 [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-04 11:56 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-05 06:02 ` RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-05 09:56 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-11 14:00 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Peter Eisentraut <peter.eisentraut@enterprisedb.com>
2021-01-12 00:59 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-12 02:09 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@gmail.com>
@ 2021-01-13 04:55 ` Fujii Masao <masao.fujii@gmail.com>
2021-01-14 05:38 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
0 siblings, 1 reply; 22+ messages in thread
From: Fujii Masao @ 2021-01-13 04:55 UTC (permalink / raw)
To: Masahiko Sawada <sawada.mshk@gmail.com>; +Cc: Peter Eisentraut <peter.eisentraut@enterprisedb.com>; pgsql-hackers
On Tue, Jan 12, 2021 at 11:09 AM Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Tue, Jan 12, 2021 at 10:00 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > On Mon, Jan 11, 2021 at 11:00 PM Peter Eisentraut
> > <peter.eisentraut@enterprisedb.com> wrote:
> > >
> > > On 2021-01-05 10:56, Masahiko Sawada wrote:
> > > > BTW according to the documentation, the options of DECLARE statement
> > > > (BINARY, INSENSITIVE, SCROLL, and NO SCROLL) are order-sensitive.
> > > >
> > > > DECLARE name [ BINARY ] [ INSENSITIVE ] [ [ NO ] SCROLL ]
> > > > CURSOR [ { WITH | WITHOUT } HOLD ] FOR query
> > > >
> > > > But I realized that these options are actually order-insensitive. For
> > > > instance, we can declare a cursor like:
> > > >
> > > > =# declare abc scroll binary cursor for select * from pg_class;
> > > > DECLARE CURSOR
> > > >
> > > > The both parser code and documentation has been unchanged from 2003.
> > > > Is it a documentation bug?
> > >
> > > According to the SQL standard, the ordering of the cursor properties is
> > > fixed. Even if the PostgreSQL parser offers more flexibility, I think
> > > we should continue to encourage writing the clauses in the standard order.
> >
> > Thanks for your comment. Agreed.
> >
> > So regarding the tab completion for DECLARE statement, perhaps it
> > would be better to follow the documentation?
>
> IMO yes because it's less confusing to make the document and
> tab-completion consistent.
I updated the patch that way. Could you review this version?
Regards,
--
Fujii Masao
Attachments:
fix_tab_complete_close_fetch_move_v6.patch (application/octet-stream, 4.8K) download
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 9dcab0d2fa..6abcbea963 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -976,6 +976,11 @@ static const SchemaQuery Query_for_list_of_statistics = {
" and pg_catalog.pg_table_is_visible(c2.oid)"\
" and c2.relispartition = 'true'"
+#define Query_for_list_of_cursors \
+" SELECT pg_catalog.quote_ident(name) "\
+" FROM pg_catalog.pg_cursors "\
+" WHERE substring(pg_catalog.quote_ident(name),1,%d)='%s'"
+
/*
* These object types were introduced later than our support cutoff of
* server version 7.4. We use the VersionedQuery infrastructure so that
@@ -2284,6 +2289,10 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH_VERSIONED_SCHEMA_QUERY(Query_for_list_of_procedures, NULL);
else if (Matches("CALL", MatchAny))
COMPLETE_WITH("(");
+/* CLOSE */
+ else if (Matches("CLOSE"))
+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors
+ " UNION SELECT 'ALL'");
/* CLUSTER */
else if (Matches("CLUSTER"))
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_clusterables, "UNION SELECT 'VERBOSE'");
@@ -3002,11 +3011,44 @@ psql_completion(const char *text, int start, int end)
" UNION SELECT 'ALL'");
/* DECLARE */
+
+ /*
+ * Complete DECLARE <name> with one of BINARY, INSENSITIVE, SCROLL, NO
+ * SCROLL, and CURSOR.
+ */
else if (Matches("DECLARE", MatchAny))
COMPLETE_WITH("BINARY", "INSENSITIVE", "SCROLL", "NO SCROLL",
"CURSOR");
+
+ /*
+ * Complete DECLARE ... <option> with other options. The PostgreSQL parser
+ * allows DECLARE options to be specified in any order. But the
+ * tab-completion follows the ordering of them that the SQL standard
+ * provides, like the syntax of DECLARE command in the documentation
+ * indicates.
+ */
+ else if (HeadMatches("DECLARE") && TailMatches("BINARY"))
+ COMPLETE_WITH("INSENSITIVE", "SCROLL", "NO SCROLL", "CURSOR");
+ else if (HeadMatches("DECLARE") && TailMatches("INSENSITIVE"))
+ COMPLETE_WITH("SCROLL", "NO SCROLL", "CURSOR");
+ else if (HeadMatches("DECLARE") && TailMatches("SCROLL"))
+ COMPLETE_WITH("CURSOR");
+ /* Complete DECLARE ... [options] NO with SCROLL */
+ else if (HeadMatches("DECLARE") && TailMatches("NO"))
+ COMPLETE_WITH("SCROLL");
+
+ /*
+ * Complete DECLARE ... CURSOR with one of WITH HOLD, WITHOUT HOLD, and
+ * FOR
+ */
else if (HeadMatches("DECLARE") && TailMatches("CURSOR"))
COMPLETE_WITH("WITH HOLD", "WITHOUT HOLD", "FOR");
+ /* Complete DECLARE ... CURSOR WITH|WITHOUT with HOLD */
+ else if (HeadMatches("DECLARE") && TailMatches("CURSOR", "WITH|WITHOUT"))
+ COMPLETE_WITH("HOLD");
+ /* Complete DECLARE ... CURSOR WITH|WITHOUT HOLD with FOR */
+ else if (HeadMatches("DECLARE") && TailMatches("CURSOR", "WITH|WITHOUT", "HOLD"))
+ COMPLETE_WITH("FOR");
/* DELETE --- can be inside EXPLAIN, RULE, etc */
/* ... despite which, only complete DELETE with FROM at start of line */
@@ -3167,15 +3209,31 @@ psql_completion(const char *text, int start, int end)
/*
* Complete FETCH with one of ABSOLUTE, BACKWARD, FORWARD, RELATIVE, ALL,
- * NEXT, PRIOR, FIRST, LAST
+ * NEXT, PRIOR, FIRST, LAST, FROM, IN, and a list of cursors
*/
else if (Matches("FETCH|MOVE"))
- COMPLETE_WITH("ABSOLUTE", "BACKWARD", "FORWARD", "RELATIVE",
- "ALL", "NEXT", "PRIOR", "FIRST", "LAST");
+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors
+ " UNION SELECT 'ABSOLUTE'"
+ " UNION SELECT 'BACKWARD'"
+ " UNION SELECT 'FORWARD'"
+ " UNION SELECT 'RELATIVE'"
+ " UNION SELECT 'ALL'"
+ " UNION SELECT 'NEXT'"
+ " UNION SELECT 'PRIOR'"
+ " UNION SELECT 'FIRST'"
+ " UNION SELECT 'LAST'"
+ " UNION SELECT 'FROM'"
+ " UNION SELECT 'IN'");
- /* Complete FETCH BACKWARD or FORWARD with one of ALL, FROM, IN */
+ /*
+ * Complete FETCH BACKWARD or FORWARD with one of ALL, FROM, IN, and a
+ * list of cursors
+ */
else if (Matches("FETCH|MOVE", "BACKWARD|FORWARD"))
- COMPLETE_WITH("ALL", "FROM", "IN");
+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors
+ " UNION SELECT 'ALL'"
+ " UNION SELECT 'FROM'"
+ " UNION SELECT 'IN'");
/*
* Complete FETCH <direction> with "FROM" or "IN". These are equivalent,
@@ -3185,7 +3243,13 @@ psql_completion(const char *text, int start, int end)
else if (Matches("FETCH|MOVE", "ABSOLUTE|BACKWARD|FORWARD|RELATIVE",
MatchAnyExcept("FROM|IN")) ||
Matches("FETCH|MOVE", "ALL|NEXT|PRIOR|FIRST|LAST"))
- COMPLETE_WITH("FROM", "IN");
+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors
+ " UNION SELECT 'FROM'"
+ " UNION SELECT 'IN'");
+ /* Complete FETCH <direction> "FROM" or "IN" with a list of cursors */
+ else if (HeadMatches("FETCH|MOVE") &&
+ TailMatches("FROM|IN"))
+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors);
/* FOREIGN DATA WRAPPER */
/* applies in ALTER/DROP FDW and in CREATE SERVER */
* Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion
2020-12-09 03:57 [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-04 11:56 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-05 06:02 ` RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-05 09:56 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-11 14:00 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Peter Eisentraut <peter.eisentraut@enterprisedb.com>
2021-01-12 00:59 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-12 02:09 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@gmail.com>
2021-01-13 04:55 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@gmail.com>
@ 2021-01-14 05:38 ` Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-14 06:43 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@oss.nttdata.com>
0 siblings, 1 reply; 22+ messages in thread
From: Masahiko Sawada @ 2021-01-14 05:38 UTC (permalink / raw)
To: Fujii Masao <masao.fujii@gmail.com>; +Cc: Peter Eisentraut <peter.eisentraut@enterprisedb.com>; pgsql-hackers
On Wed, Jan 13, 2021 at 1:55 PM Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Tue, Jan 12, 2021 at 11:09 AM Fujii Masao <masao.fujii@gmail.com> wrote:
> >
> > On Tue, Jan 12, 2021 at 10:00 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > >
> > > On Mon, Jan 11, 2021 at 11:00 PM Peter Eisentraut
> > > <peter.eisentraut@enterprisedb.com> wrote:
> > > >
> > > > On 2021-01-05 10:56, Masahiko Sawada wrote:
> > > > > BTW according to the documentation, the options of DECLARE statement
> > > > > (BINARY, INSENSITIVE, SCROLL, and NO SCROLL) are order-sensitive.
> > > > >
> > > > > DECLARE name [ BINARY ] [ INSENSITIVE ] [ [ NO ] SCROLL ]
> > > > > CURSOR [ { WITH | WITHOUT } HOLD ] FOR query
> > > > >
> > > > > But I realized that these options are actually order-insensitive. For
> > > > > instance, we can declare a cursor like:
> > > > >
> > > > > =# declare abc scroll binary cursor for select * from pg_class;
> > > > > DECLARE CURSOR
> > > > >
> > > > > The both parser code and documentation has been unchanged from 2003.
> > > > > Is it a documentation bug?
> > > >
> > > > According to the SQL standard, the ordering of the cursor properties is
> > > > fixed. Even if the PostgreSQL parser offers more flexibility, I think
> > > > we should continue to encourage writing the clauses in the standard order.
> > >
> > > Thanks for your comment. Agreed.
> > >
> > > So regarding the tab completion for DECLARE statement, perhaps it
> > > would be better to follow the documentation?
> >
> > IMO yes because it's less confusing to make the document and
> > tab-completion consistent.
Agreed.
>
> I updated the patch that way. Could you review this version?
Thank you for updating the patch. Looks good to me.
Regards,
--
Masahiko Sawada
EnterpriseDB: https://www.enterprisedb.com/
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion
2020-12-09 03:57 [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-04 11:56 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-05 06:02 ` RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-05 09:56 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-11 14:00 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Peter Eisentraut <peter.eisentraut@enterprisedb.com>
2021-01-12 00:59 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-12 02:09 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@gmail.com>
2021-01-13 04:55 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Fujii Masao <masao.fujii@gmail.com>
2021-01-14 05:38 ` Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Masahiko Sawada <sawada.mshk@gmail.com>
@ 2021-01-14 06:43 ` Fujii Masao <masao.fujii@oss.nttdata.com>
0 siblings, 0 replies; 22+ messages in thread
From: Fujii Masao @ 2021-01-14 06:43 UTC (permalink / raw)
To: Masahiko Sawada <sawada.mshk@gmail.com>; Fujii Masao <masao.fujii@gmail.com>; +Cc: Peter Eisentraut <peter.eisentraut@enterprisedb.com>; pgsql-hackers
On 2021/01/14 14:38, Masahiko Sawada wrote:
> On Wed, Jan 13, 2021 at 1:55 PM Fujii Masao <masao.fujii@gmail.com> wrote:
>>
>> On Tue, Jan 12, 2021 at 11:09 AM Fujii Masao <masao.fujii@gmail.com> wrote:
>>>
>>> On Tue, Jan 12, 2021 at 10:00 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>>>>
>>>> On Mon, Jan 11, 2021 at 11:00 PM Peter Eisentraut
>>>> <peter.eisentraut@enterprisedb.com> wrote:
>>>>>
>>>>> On 2021-01-05 10:56, Masahiko Sawada wrote:
>>>>>> BTW according to the documentation, the options of DECLARE statement
>>>>>> (BINARY, INSENSITIVE, SCROLL, and NO SCROLL) are order-sensitive.
>>>>>>
>>>>>> DECLARE name [ BINARY ] [ INSENSITIVE ] [ [ NO ] SCROLL ]
>>>>>> CURSOR [ { WITH | WITHOUT } HOLD ] FOR query
>>>>>>
>>>>>> But I realized that these options are actually order-insensitive. For
>>>>>> instance, we can declare a cursor like:
>>>>>>
>>>>>> =# declare abc scroll binary cursor for select * from pg_class;
>>>>>> DECLARE CURSOR
>>>>>>
>>>>>> The both parser code and documentation has been unchanged from 2003.
>>>>>> Is it a documentation bug?
>>>>>
>>>>> According to the SQL standard, the ordering of the cursor properties is
>>>>> fixed. Even if the PostgreSQL parser offers more flexibility, I think
>>>>> we should continue to encourage writing the clauses in the standard order.
>>>>
>>>> Thanks for your comment. Agreed.
>>>>
>>>> So regarding the tab completion for DECLARE statement, perhaps it
>>>> would be better to follow the documentation?
>>>
>>> IMO yes because it's less confusing to make the document and
>>> tab-completion consistent.
>
> Agreed.
>
>>
>> I updated the patch that way. Could you review this version?
>
> Thank you for updating the patch. Looks good to me.
Pushed. Thanks!
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
^ permalink raw reply [nested|flat] 22+ messages in thread
end of thread, other threads:[~2021-01-14 06:43 UTC | newest]
Thread overview: 22+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-12-09 03:57 [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion Shinya11.Kato@nttdata.com
2021-01-04 11:56 ` Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-05 06:02 ` Shinya11.Kato@nttdata.com
2021-01-05 09:08 ` Fujii Masao <masao.fujii@oss.nttdata.com>
2021-01-06 02:13 ` Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-06 02:33 ` Fujii Masao <masao.fujii@oss.nttdata.com>
2021-01-06 06:36 ` Shinya11.Kato@nttdata.com
2021-01-07 01:01 ` Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-07 01:59 ` Fujii Masao <masao.fujii@oss.nttdata.com>
2021-01-07 03:42 ` Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-07 04:30 ` Fujii Masao <masao.fujii@oss.nttdata.com>
2021-01-07 06:53 ` Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-07 08:28 ` Shinya11.Kato@nttdata.com
2021-01-07 12:32 ` Fujii Masao <masao.fujii@oss.nttdata.com>
2021-01-11 05:39 ` Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-05 09:56 ` Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-11 14:00 ` Peter Eisentraut <peter.eisentraut@enterprisedb.com>
2021-01-12 00:59 ` Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-12 02:09 ` Fujii Masao <masao.fujii@gmail.com>
2021-01-13 04:55 ` Fujii Masao <masao.fujii@gmail.com>
2021-01-14 05:38 ` Masahiko Sawada <sawada.mshk@gmail.com>
2021-01-14 06:43 ` Fujii Masao <masao.fujii@oss.nttdata.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox