psql: tab-completion support for COPY ... TO/FROM STDIN, STDOUT, and PROGRAM

On Thu, 5 Jun 2025 10:08:35 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

Hi,

Currently, tab completion for COPY only suggests filenames after TO or
FROM, even though STDIN, STDOUT, and PROGRAM are also valid syntax options.

I'd like to propose improving the tab completion behavior as described in
the subject, so that these keywords are suggested appropriately, and filenames
are offered as potential command names after the PROGRAM keyword.

I've attached this proposal as a patch series with the following three parts:

I'm sorry but the previous patches were accidentally broken and didn't work.
I've attached fixed patches.

0001: Refactor match_previous_words() to remove direct use of rl_completion_matches()

This is a preparatory cleanup. Most completions in match_previous_words() already use
COMPLETE_WITH* macros, which wrap rl_completion_matches(). However, some direct calls
still remain.

This patch replaces the remaining direct calls with COMPLETE_WITH_FILES or
COMPLETE_WITH_GENERATOR, improving consistency and readability.

0002: Add tab completion support for COPY ... TO/FROM STDIN, STDOUT, and PROGRAM

This is the main patch. It extends tab completion to suggest STDIN, STDOUT, and PROGRAM
after TO or FROM. After PROGRAM, filenames are suggested as possible command names.

To support this, a new macro COMPLETE_WITH_FILES_PLUS is introduced. This allows
combining literal keywords with filename suggestions in the completion list.

0003: Improve tab completion for COPY option lists

Currently, only the first option in a parenthesized list is suggested during completion,
and nothing is suggested after a comma.

This patch enables suggestions after each comma, improving usability when specifying
multiple options.

Although not directly related to the main proposal, I believe this is a helpful enhancement
to COPY tab completion and included it here for completeness.

I’d appreciate your review and feedback on this series.

Best regards,
Yugo Nagata

--
Yugo Nagata <nagata@sraoss.co.jp>

--
Yugo Nagata <nagata@sraoss.co.jp>

On Thu, 5 Jun 2025 16:52:00 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Thu, 5 Jun 2025 10:08:35 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

Hi,

Currently, tab completion for COPY only suggests filenames after TO or
FROM, even though STDIN, STDOUT, and PROGRAM are also valid syntax options.

I'd like to propose improving the tab completion behavior as described in
the subject, so that these keywords are suggested appropriately, and filenames
are offered as potential command names after the PROGRAM keyword.

I've attached this proposal as a patch series with the following three parts:

I'm sorry but the previous patches were accidentally broken and didn't work.
I've attached fixed patches.

0001: Refactor match_previous_words() to remove direct use of rl_completion_matches()

This is a preparatory cleanup. Most completions in match_previous_words() already use
COMPLETE_WITH* macros, which wrap rl_completion_matches(). However, some direct calls
still remain.

This patch replaces the remaining direct calls with COMPLETE_WITH_FILES or
COMPLETE_WITH_GENERATOR, improving consistency and readability.

0002: Add tab completion support for COPY ... TO/FROM STDIN, STDOUT, and PROGRAM

This is the main patch. It extends tab completion to suggest STDIN, STDOUT, and PROGRAM
after TO or FROM. After PROGRAM, filenames are suggested as possible command names.

To support this, a new macro COMPLETE_WITH_FILES_PLUS is introduced. This allows
combining literal keywords with filename suggestions in the completion list.

0003: Improve tab completion for COPY option lists

Currently, only the first option in a parenthesized list is suggested during completion,
and nothing is suggested after a comma.

This patch enables suggestions after each comma, improving usability when specifying
multiple options.

Although not directly related to the main proposal, I believe this is a helpful enhancement
to COPY tab completion and included it here for completeness.

I’d appreciate your review and feedback on this series.

I've attached rebased patches.

Best regards,
Yugo Nagata

--
Yugo Nagata <nagata@sraoss.co.jp>

On Tue, 17 Jun 2025 00:08:32 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Thu, 5 Jun 2025 16:52:00 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Thu, 5 Jun 2025 10:08:35 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

Hi,

Currently, tab completion for COPY only suggests filenames after TO or
FROM, even though STDIN, STDOUT, and PROGRAM are also valid syntax options.

I'd like to propose improving the tab completion behavior as described in
the subject, so that these keywords are suggested appropriately, and filenames
are offered as potential command names after the PROGRAM keyword.

I've attached this proposal as a patch series with the following three parts:

I'm sorry but the previous patches were accidentally broken and didn't work.
I've attached fixed patches.

0001: Refactor match_previous_words() to remove direct use of rl_completion_matches()

This is a preparatory cleanup. Most completions in match_previous_words() already use
COMPLETE_WITH* macros, which wrap rl_completion_matches(). However, some direct calls
still remain.

This patch replaces the remaining direct calls with COMPLETE_WITH_FILES or
COMPLETE_WITH_GENERATOR, improving consistency and readability.

0002: Add tab completion support for COPY ... TO/FROM STDIN, STDOUT, and PROGRAM

This is the main patch. It extends tab completion to suggest STDIN, STDOUT, and PROGRAM
after TO or FROM. After PROGRAM, filenames are suggested as possible command names.

To support this, a new macro COMPLETE_WITH_FILES_PLUS is introduced. This allows
combining literal keywords with filename suggestions in the completion list.

0003: Improve tab completion for COPY option lists

Currently, only the first option in a parenthesized list is suggested during completion,
and nothing is suggested after a comma.

This patch enables suggestions after each comma, improving usability when specifying
multiple options.

Although not directly related to the main proposal, I believe this is a helpful enhancement
to COPY tab completion and included it here for completeness.

I’d appreciate your review and feedback on this series.

I've attached rebased patches.

Regards,
Yugo Nagata

--
Yugo Nagata <nagata@sraoss.co.jp>

On Thu, 17 Jul 2025 10:57:36 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Tue, 17 Jun 2025 00:08:32 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Thu, 5 Jun 2025 16:52:00 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Thu, 5 Jun 2025 10:08:35 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

Hi,

Currently, tab completion for COPY only suggests filenames after TO or
FROM, even though STDIN, STDOUT, and PROGRAM are also valid syntax options.

I'd like to propose improving the tab completion behavior as described in
the subject, so that these keywords are suggested appropriately, and filenames
are offered as potential command names after the PROGRAM keyword.

I've attached this proposal as a patch series with the following three parts:

I'm sorry but the previous patches were accidentally broken and didn't work.
I've attached fixed patches.

0001: Refactor match_previous_words() to remove direct use of rl_completion_matches()

This is a preparatory cleanup. Most completions in match_previous_words() already use
COMPLETE_WITH* macros, which wrap rl_completion_matches(). However, some direct calls
still remain.

This patch replaces the remaining direct calls with COMPLETE_WITH_FILES or
COMPLETE_WITH_GENERATOR, improving consistency and readability.

0002: Add tab completion support for COPY ... TO/FROM STDIN, STDOUT, and PROGRAM

This is the main patch. It extends tab completion to suggest STDIN, STDOUT, and PROGRAM
after TO or FROM. After PROGRAM, filenames are suggested as possible command names.

To support this, a new macro COMPLETE_WITH_FILES_PLUS is introduced. This allows
combining literal keywords with filename suggestions in the completion list.

0003: Improve tab completion for COPY option lists

Currently, only the first option in a parenthesized list is suggested during completion,
and nothing is suggested after a comma.

This patch enables suggestions after each comma, improving usability when specifying
multiple options.

Although not directly related to the main proposal, I believe this is a helpful enhancement
to COPY tab completion and included it here for completeness.

I’d appreciate your review and feedback on this series.

The previous patch was broken failed to complie since I missed following
the required format of if-conditions in match_previous_words().
I've attached update patches.

Regards,
Yugo Nagata

--
Yugo Nagata <nagata@sraoss.co.jp>

On Fri, 18 Jul 2025 at 12:49, Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Thu, 17 Jul 2025 10:57:36 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Tue, 17 Jun 2025 00:08:32 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Thu, 5 Jun 2025 16:52:00 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Thu, 5 Jun 2025 10:08:35 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

Hi,

Currently, tab completion for COPY only suggests filenames after TO or
FROM, even though STDIN, STDOUT, and PROGRAM are also valid syntax options.

I'd like to propose improving the tab completion behavior as described in
the subject, so that these keywords are suggested appropriately, and filenames
are offered as potential command names after the PROGRAM keyword.

I've attached this proposal as a patch series with the following three parts:

I'm sorry but the previous patches were accidentally broken and didn't work.
I've attached fixed patches.

0001: Refactor match_previous_words() to remove direct use of rl_completion_matches()

This is a preparatory cleanup. Most completions in match_previous_words() already use
COMPLETE_WITH* macros, which wrap rl_completion_matches(). However, some direct calls
still remain.

This patch replaces the remaining direct calls with COMPLETE_WITH_FILES or
COMPLETE_WITH_GENERATOR, improving consistency and readability.

0002: Add tab completion support for COPY ... TO/FROM STDIN, STDOUT, and PROGRAM

This is the main patch. It extends tab completion to suggest STDIN, STDOUT, and PROGRAM
after TO or FROM. After PROGRAM, filenames are suggested as possible command names.

To support this, a new macro COMPLETE_WITH_FILES_PLUS is introduced. This allows
combining literal keywords with filename suggestions in the completion list.

0003: Improve tab completion for COPY option lists

Currently, only the first option in a parenthesized list is suggested during completion,
and nothing is suggested after a comma.

This patch enables suggestions after each comma, improving usability when specifying
multiple options.

Although not directly related to the main proposal, I believe this is a helpful enhancement
to COPY tab completion and included it here for completeness.

I’d appreciate your review and feedback on this series.

The previous patch was broken failed to complie since I missed following
the required format of if-conditions in match_previous_words().
I've attached update patches.

Regards,
Yugo Nagata

--
Yugo Nagata <nagata@sraoss.co.jp>

Hi! This tab completion support is indeed very useful. Thanks for
working on this.
I spotted potential improvement here: current v5 does not support COPY
completion for a pattern I do frequently use:
specifying COPY options without WITH clause e.g. "copy yy from
'/home/reshke/cpg/csv.csv' <tab>" Is not completed with BINARY, CSV,
DELIMITER etc.

--
Best regards,
Kirill Reshke

Hi

Currently, tab completion for COPY only suggests filenames after TO or
FROM, even though STDIN, STDOUT, and PROGRAM are also valid syntax options.

I'd like to propose improving the tab completion behavior as described in
the subject, so that these keywords are suggested appropriately, and filenames
are offered as potential command names after the PROGRAM keyword.

I've attached this proposal as a patch series with the following three parts:

I'm sorry but the previous patches were accidentally broken and didn't work.
I've attached fixed patches.

0001: Refactor match_previous_words() to remove direct use of rl_completion_matches()

This is a preparatory cleanup. Most completions in match_previous_words() already use
COMPLETE_WITH* macros, which wrap rl_completion_matches(). However, some direct calls
still remain.

This patch replaces the remaining direct calls with COMPLETE_WITH_FILES or
COMPLETE_WITH_GENERATOR, improving consistency and readability.

0002: Add tab completion support for COPY ... TO/FROM STDIN, STDOUT, and PROGRAM

This is the main patch. It extends tab completion to suggest STDIN, STDOUT, and PROGRAM
after TO or FROM. After PROGRAM, filenames are suggested as possible command names.

To support this, a new macro COMPLETE_WITH_FILES_PLUS is introduced. This allows
combining literal keywords with filename suggestions in the completion list.

0003: Improve tab completion for COPY option lists

Currently, only the first option in a parenthesized list is suggested during completion,
and nothing is suggested after a comma.

This patch enables suggestions after each comma, improving usability when specifying
multiple options.

Although not directly related to the main proposal, I believe this is a helpful enhancement
to COPY tab completion and included it here for completeness.

I’d appreciate your review and feedback on this series.

Hi! This tab completion support is indeed very useful. Thanks for
working on this.

Thank you for your feedback!

I spotted potential improvement here: current v5 does not support COPY
completion for a pattern I do frequently use:
specifying COPY options without WITH clause e.g. "copy yy from
'/home/reshke/cpg/csv.csv' <tab>" Is not completed with BINARY, CSV,
DELIMITER etc.

This is the old syntax and is supported for backward compatibility
(commit 923413ac6d3), but I’m not sure it’s worth supporting in tab
completion. Perhaps it would be better to discuss this point in a
separate thread.

Regards,
Yugo Nagata

--
Yugo Nagata <nagata@sraoss.co.jp>

On Thu, 28 Aug 2025 at 12:22, Yugo Nagata <nagata@sraoss.co.jp> wrote:

Hi! This tab completion support is indeed very useful. Thanks for
working on this.

Thank you for your feedback!

I spotted potential improvement here: current v5 does not support COPY
completion for a pattern I do frequently use:
specifying COPY options without WITH clause e.g. "copy yy from
'/home/reshke/cpg/csv.csv' <tab>" Is not completed with BINARY, CSV,
DELIMITER etc.

This is the old syntax and is supported for backward compatibility
(commit 923413ac6d3), but I’m not sure it’s worth supporting in tab
completion. Perhaps it would be better to discuss this point in a
separate thread.

I see your point. However I do not think that old syntax is a show-stopper here.
current psql (on HEAD) has TAB support for archaic patterns like START
<tab> completed with TRANSACTION and ABORT W<tab> completed with WORK.
And I do think that the case I use in COPY (and propose to support
thus) is more frequent than these two. I came across this thread while
looking to support my usecase.
This does not make you obligated to support this in this patch series.
I can even send v5-0004 with this support in the thread if that's
okay.

--
Best regards,
Kirill Reshke

On Fri, Jul 18, 2025 at 12:49 AM Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Thu, 17 Jul 2025 10:57:36 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Tue, 17 Jun 2025 00:08:32 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Thu, 5 Jun 2025 16:52:00 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Thu, 5 Jun 2025 10:08:35 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

Hi,

Currently, tab completion for COPY only suggests filenames after TO or
FROM, even though STDIN, STDOUT, and PROGRAM are also valid syntax options.

I'd like to propose improving the tab completion behavior as described in
the subject, so that these keywords are suggested appropriately, and filenames
are offered as potential command names after the PROGRAM keyword.

I've attached this proposal as a patch series with the following three parts:

I'm sorry but the previous patches were accidentally broken and didn't work.
I've attached fixed patches.

0001: Refactor match_previous_words() to remove direct use of rl_completion_matches()

This is a preparatory cleanup. Most completions in match_previous_words() already use
COMPLETE_WITH* macros, which wrap rl_completion_matches(). However, some direct calls
still remain.

This patch replaces the remaining direct calls with COMPLETE_WITH_FILES or
COMPLETE_WITH_GENERATOR, improving consistency and readability.

0002: Add tab completion support for COPY ... TO/FROM STDIN, STDOUT, and PROGRAM

This is the main patch. It extends tab completion to suggest STDIN, STDOUT, and PROGRAM
after TO or FROM. After PROGRAM, filenames are suggested as possible command names.

To support this, a new macro COMPLETE_WITH_FILES_PLUS is introduced. This allows
combining literal keywords with filename suggestions in the completion list.

0003: Improve tab completion for COPY option lists

Currently, only the first option in a parenthesized list is suggested during completion,
and nothing is suggested after a comma.

This patch enables suggestions after each comma, improving usability when specifying
multiple options.

Although not directly related to the main proposal, I believe this is a helpful enhancement
to COPY tab completion and included it here for completeness.

I’d appreciate your review and feedback on this series.

The previous patch was broken failed to complie since I missed following
the required format of if-conditions in match_previous_words().
I've attached update patches.

I agree with the basic direction of the patches. Here are some
comments on the first two patches:

v5-0001-Refactor-match_previous_words-to-remove-direct-us.patch:

---
+#define COMPLETE_WITH_GENERATOR(function) \
+   matches = rl_completion_matches(text, function)

I think it would be clearer if we use 'generator' or 'genfunc' instead
of 'function' as a macro argument.

v5-0002-Add-tab-completion-support-for-COPY-.-TO-FROM-STD.patch:

---
+        /* Complete COPY|\copy <sth> FROM|TO PROGRAM command */
+        else if (Matches("COPY|\\copy", MatchAny, "FROM|TO", "PROGRAM"))
+                COMPLETE_WITH_FILES("", HeadMatches("COPY"));  /*
COPY requires quoted filename */

Why does it complete the query with files names even after 'PROGRAM'?

---
+static char *
+_complete_from_files(const char *text, int state)
 {

I think the comments of complete_from_files() should be moved to this
new function. For instance, the comments starts with:

* This function wraps rl_filename_completion_function() to strip quotes from
* the input before searching for matches and to quote any matches for which
* the consuming command will require it.

But complete_from_files() function no longer calls
rl_filename_completion_function().

---
-   /* Complete COPY <sth> FROM <sth> */
-   else if (Matches("COPY|\\copy", MatchAny, "FROM", MatchAny))
+   /* Complete COPY <sth> FROM [PROGRAM] <sth> */
+   else if (Matches("COPY|\\copy", MatchAny, "FROM",
MatchAnyExcept("PROGRAM")) ||
+            Matches("COPY|\\copy", MatchAny, "FROM", "PROGRAM", MatchAny))

I see this kind of conversion many places in the patch; convert one
condition with MatchAny into two conditions with
MatchAnyExcept("PROGRAM") and '"PROGRAM", MatchAny'. How about
simplifying it using MatchAnyN. For example,

else if (Matches("COPY|\\copy", MatchAny, "FROM", MatchAny, MatchAnyN))

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

On Wed, 17 Sep 2025 12:53:10 -0700
Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Fri, Jul 18, 2025 at 12:49 AM Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Thu, 17 Jul 2025 10:57:36 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Tue, 17 Jun 2025 00:08:32 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Thu, 5 Jun 2025 16:52:00 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Thu, 5 Jun 2025 10:08:35 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

Hi,

Currently, tab completion for COPY only suggests filenames after TO or
FROM, even though STDIN, STDOUT, and PROGRAM are also valid syntax options.

I'd like to propose improving the tab completion behavior as described in
the subject, so that these keywords are suggested appropriately, and filenames
are offered as potential command names after the PROGRAM keyword.

I've attached this proposal as a patch series with the following three parts:

I'm sorry but the previous patches were accidentally broken and didn't work.
I've attached fixed patches.

0001: Refactor match_previous_words() to remove direct use of rl_completion_matches()

This is a preparatory cleanup. Most completions in match_previous_words() already use
COMPLETE_WITH* macros, which wrap rl_completion_matches(). However, some direct calls
still remain.

This patch replaces the remaining direct calls with COMPLETE_WITH_FILES or
COMPLETE_WITH_GENERATOR, improving consistency and readability.

0002: Add tab completion support for COPY ... TO/FROM STDIN, STDOUT, and PROGRAM

This is the main patch. It extends tab completion to suggest STDIN, STDOUT, and PROGRAM
after TO or FROM. After PROGRAM, filenames are suggested as possible command names.

To support this, a new macro COMPLETE_WITH_FILES_PLUS is introduced. This allows
combining literal keywords with filename suggestions in the completion list.

0003: Improve tab completion for COPY option lists

Currently, only the first option in a parenthesized list is suggested during completion,
and nothing is suggested after a comma.

This patch enables suggestions after each comma, improving usability when specifying
multiple options.

Although not directly related to the main proposal, I believe this is a helpful enhancement
to COPY tab completion and included it here for completeness.

I’d appreciate your review and feedback on this series.

The previous patch was broken failed to complie since I missed following
the required format of if-conditions in match_previous_words().
I've attached update patches.

I agree with the basic direction of the patches. Here are some
comments on the first two patches:

Thank you for reviewing it.
I've attached an updated patch.

v5-0001-Refactor-match_previous_words-to-remove-direct-us.patch:

---
+#define COMPLETE_WITH_GENERATOR(function) \
+   matches = rl_completion_matches(text, function)

I think it would be clearer if we use 'generator' or 'genfunc' instead
of 'function' as a macro argument.

I fixed it to use 'generator'.

v5-0002-Add-tab-completion-support-for-COPY-.-TO-FROM-STD.patch:

---
+        /* Complete COPY|\copy <sth> FROM|TO PROGRAM command */
+        else if (Matches("COPY|\\copy", MatchAny, "FROM|TO", "PROGRAM"))
+                COMPLETE_WITH_FILES("", HeadMatches("COPY"));  /*
COPY requires quoted filename */

Why does it complete the query with files names even after 'PROGRAM'?

Users can specify the command by giving a filename with an absolute or
relative path, so I think it makes sense to allow filename completion
after PROGRAM.

---
+static char *
+_complete_from_files(const char *text, int state)
{

I think the comments of complete_from_files() should be moved to this
new function. For instance, the comments starts with:

* This function wraps rl_filename_completion_function() to strip quotes from
* the input before searching for matches and to quote any matches for which
* the consuming command will require it.

But complete_from_files() function no longer calls
rl_filename_completion_function().

I moved the comments to the top of _complete_from_files() and added a new
comment for complete_from_files() to describe that it is a wrapper of the
former.

---
-   /* Complete COPY <sth> FROM <sth> */
-   else if (Matches("COPY|\\copy", MatchAny, "FROM", MatchAny))
+   /* Complete COPY <sth> FROM [PROGRAM] <sth> */
+   else if (Matches("COPY|\\copy", MatchAny, "FROM",
MatchAnyExcept("PROGRAM")) ||
+            Matches("COPY|\\copy", MatchAny, "FROM", "PROGRAM", MatchAny))

I see this kind of conversion many places in the patch; convert one
condition with MatchAny into two conditions with
MatchAnyExcept("PROGRAM") and '"PROGRAM", MatchAny'. How about
simplifying it using MatchAnyN. For example,

else if (Matches("COPY|\\copy", MatchAny, "FROM", MatchAny, MatchAnyN))

We could simplify this by using MatchAnyN, but doing so would cause "WITH ("
or "WHERE" to be suggested after "WITH (...)", even though that is not allowed
by the syntax. This could be misleading for users, so I wonder whether it is
worth adding a bit of complexity to prevent possible confusion.

Regards,
Yugo Nagata

--
Yugo Nagata <nagata@sraoss.co.jp>

On Thu, 18 Sep 2025 12:05:06 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Wed, 17 Sep 2025 12:53:10 -0700
Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Fri, Jul 18, 2025 at 12:49 AM Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Thu, 17 Jul 2025 10:57:36 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Tue, 17 Jun 2025 00:08:32 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Thu, 5 Jun 2025 16:52:00 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Thu, 5 Jun 2025 10:08:35 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

Hi,

Currently, tab completion for COPY only suggests filenames after TO or
FROM, even though STDIN, STDOUT, and PROGRAM are also valid syntax options.

I'd like to propose improving the tab completion behavior as described in
the subject, so that these keywords are suggested appropriately, and filenames
are offered as potential command names after the PROGRAM keyword.

I've attached this proposal as a patch series with the following three parts:

I'm sorry but the previous patches were accidentally broken and didn't work.
I've attached fixed patches.

0001: Refactor match_previous_words() to remove direct use of rl_completion_matches()

This is a preparatory cleanup. Most completions in match_previous_words() already use
COMPLETE_WITH* macros, which wrap rl_completion_matches(). However, some direct calls
still remain.

This patch replaces the remaining direct calls with COMPLETE_WITH_FILES or
COMPLETE_WITH_GENERATOR, improving consistency and readability.

0002: Add tab completion support for COPY ... TO/FROM STDIN, STDOUT, and PROGRAM

This is the main patch. It extends tab completion to suggest STDIN, STDOUT, and PROGRAM
after TO or FROM. After PROGRAM, filenames are suggested as possible command names.

To support this, a new macro COMPLETE_WITH_FILES_PLUS is introduced. This allows
combining literal keywords with filename suggestions in the completion list.

0003: Improve tab completion for COPY option lists

Currently, only the first option in a parenthesized list is suggested during completion,
and nothing is suggested after a comma.

This patch enables suggestions after each comma, improving usability when specifying
multiple options.

Although not directly related to the main proposal, I believe this is a helpful enhancement
to COPY tab completion and included it here for completeness.

I’d appreciate your review and feedback on this series.

The previous patch was broken failed to complie since I missed following
the required format of if-conditions in match_previous_words().
I've attached update patches.

I agree with the basic direction of the patches. Here are some
comments on the first two patches:

Thank you for reviewing it.
I've attached an updated patch.

v5-0001-Refactor-match_previous_words-to-remove-direct-us.patch:

---
+#define COMPLETE_WITH_GENERATOR(function) \
+   matches = rl_completion_matches(text, function)

I think it would be clearer if we use 'generator' or 'genfunc' instead
of 'function' as a macro argument.

I fixed it to use 'generator'.

v5-0002-Add-tab-completion-support-for-COPY-.-TO-FROM-STD.patch:

---
+        /* Complete COPY|\copy <sth> FROM|TO PROGRAM command */
+        else if (Matches("COPY|\\copy", MatchAny, "FROM|TO", "PROGRAM"))
+                COMPLETE_WITH_FILES("", HeadMatches("COPY"));  /*
COPY requires quoted filename */

Why does it complete the query with files names even after 'PROGRAM'?

Users can specify the command by giving a filename with an absolute or
relative path, so I think it makes sense to allow filename completion
after PROGRAM.

---
+static char *
+_complete_from_files(const char *text, int state)
{

I think the comments of complete_from_files() should be moved to this
new function. For instance, the comments starts with:

* This function wraps rl_filename_completion_function() to strip quotes from
* the input before searching for matches and to quote any matches for which
* the consuming command will require it.

But complete_from_files() function no longer calls
rl_filename_completion_function().

I moved the comments to the top of _complete_from_files() and added a new
comment for complete_from_files() to describe that it is a wrapper of the
former.

---
-   /* Complete COPY <sth> FROM <sth> */
-   else if (Matches("COPY|\\copy", MatchAny, "FROM", MatchAny))
+   /* Complete COPY <sth> FROM [PROGRAM] <sth> */
+   else if (Matches("COPY|\\copy", MatchAny, "FROM",
MatchAnyExcept("PROGRAM")) ||
+            Matches("COPY|\\copy", MatchAny, "FROM", "PROGRAM", MatchAny))

I see this kind of conversion many places in the patch; convert one
condition with MatchAny into two conditions with
MatchAnyExcept("PROGRAM") and '"PROGRAM", MatchAny'. How about
simplifying it using MatchAnyN. For example,

else if (Matches("COPY|\\copy", MatchAny, "FROM", MatchAny, MatchAnyN))

We could simplify this by using MatchAnyN, but doing so would cause "WITH ("
or "WHERE" to be suggested after "WITH (...)", even though that is not allowed
by the syntax. This could be misleading for users, so I wonder whether it is
worth adding a bit of complexity to prevent possible confusion.

There was a mistake in the previous statement: "WHERE" appearing after "WITH (...)"
is actually correct. However, this also results in "WITH" being suggested after
"WHERE", which is not permitted by the syntax.

Regards,
Yugo Nagata

--
Yugo Nagata <nagata@sraoss.co.jp>

On Wed, Sep 17, 2025 at 8:27 PM Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Thu, 18 Sep 2025 12:05:06 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Wed, 17 Sep 2025 12:53:10 -0700
Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Fri, Jul 18, 2025 at 12:49 AM Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Thu, 17 Jul 2025 10:57:36 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Tue, 17 Jun 2025 00:08:32 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Thu, 5 Jun 2025 16:52:00 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Thu, 5 Jun 2025 10:08:35 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

Hi,

Currently, tab completion for COPY only suggests filenames after TO or
FROM, even though STDIN, STDOUT, and PROGRAM are also valid syntax options.

I'd like to propose improving the tab completion behavior as described in
the subject, so that these keywords are suggested appropriately, and filenames
are offered as potential command names after the PROGRAM keyword.

I've attached this proposal as a patch series with the following three parts:

I'm sorry but the previous patches were accidentally broken and didn't work.
I've attached fixed patches.

0001: Refactor match_previous_words() to remove direct use of rl_completion_matches()

This is a preparatory cleanup. Most completions in match_previous_words() already use
COMPLETE_WITH* macros, which wrap rl_completion_matches(). However, some direct calls
still remain.

This patch replaces the remaining direct calls with COMPLETE_WITH_FILES or
COMPLETE_WITH_GENERATOR, improving consistency and readability.

0002: Add tab completion support for COPY ... TO/FROM STDIN, STDOUT, and PROGRAM

This is the main patch. It extends tab completion to suggest STDIN, STDOUT, and PROGRAM
after TO or FROM. After PROGRAM, filenames are suggested as possible command names.

To support this, a new macro COMPLETE_WITH_FILES_PLUS is introduced. This allows
combining literal keywords with filename suggestions in the completion list.

0003: Improve tab completion for COPY option lists

Currently, only the first option in a parenthesized list is suggested during completion,
and nothing is suggested after a comma.

This patch enables suggestions after each comma, improving usability when specifying
multiple options.

Although not directly related to the main proposal, I believe this is a helpful enhancement
to COPY tab completion and included it here for completeness.

I’d appreciate your review and feedback on this series.

The previous patch was broken failed to complie since I missed following
the required format of if-conditions in match_previous_words().
I've attached update patches.

I agree with the basic direction of the patches. Here are some
comments on the first two patches:

Thank you for reviewing it.
I've attached an updated patch.

Thank you for updating the patches!

v5-0001-Refactor-match_previous_words-to-remove-direct-us.patch:

---
+#define COMPLETE_WITH_GENERATOR(function) \
+   matches = rl_completion_matches(text, function)

I think it would be clearer if we use 'generator' or 'genfunc' instead
of 'function' as a macro argument.

I fixed it to use 'generator'.

LGTM. I've pushed the 0001 patch.

v5-0002-Add-tab-completion-support-for-COPY-.-TO-FROM-STD.patch:

---
+        /* Complete COPY|\copy <sth> FROM|TO PROGRAM command */
+        else if (Matches("COPY|\\copy", MatchAny, "FROM|TO", "PROGRAM"))
+                COMPLETE_WITH_FILES("", HeadMatches("COPY"));  /*
COPY requires quoted filename */

Why does it complete the query with files names even after 'PROGRAM'?

Users can specify the command by giving a filename with an absolute or
relative path, so I think it makes sense to allow filename completion
after PROGRAM.

Agreed.

---
+static char *
+_complete_from_files(const char *text, int state)
{

I think the comments of complete_from_files() should be moved to this
new function. For instance, the comments starts with:

* This function wraps rl_filename_completion_function() to strip quotes from
* the input before searching for matches and to quote any matches for which
* the consuming command will require it.

But complete_from_files() function no longer calls
rl_filename_completion_function().

I moved the comments to the top of _complete_from_files() and added a new
comment for complete_from_files() to describe that it is a wrapper of the
former.

---
-   /* Complete COPY <sth> FROM <sth> */
-   else if (Matches("COPY|\\copy", MatchAny, "FROM", MatchAny))
+   /* Complete COPY <sth> FROM [PROGRAM] <sth> */
+   else if (Matches("COPY|\\copy", MatchAny, "FROM",
MatchAnyExcept("PROGRAM")) ||
+            Matches("COPY|\\copy", MatchAny, "FROM", "PROGRAM", MatchAny))

I see this kind of conversion many places in the patch; convert one
condition with MatchAny into two conditions with
MatchAnyExcept("PROGRAM") and '"PROGRAM", MatchAny'. How about
simplifying it using MatchAnyN. For example,

else if (Matches("COPY|\\copy", MatchAny, "FROM", MatchAny, MatchAnyN))

We could simplify this by using MatchAnyN, but doing so would cause "WITH ("
or "WHERE" to be suggested after "WITH (...)", even though that is not allowed
by the syntax. This could be misleading for users, so I wonder whether it is
worth adding a bit of complexity to prevent possible confusion.

There was a mistake in the previous statement: "WHERE" appearing after "WITH (...)"
is actually correct. However, this also results in "WITH" being suggested after
"WHERE", which is not permitted by the syntax.

True. How about other places? That is, where we check the completion
after "WITH (". For example:

-   /* Complete COPY <sth> TO filename WITH ( */
-   else if (Matches("COPY|\\copy", MatchAny, "TO", MatchAny, "WITH", "("))
+   /* Complete COPY <sth> TO [PROGRAM] filename WITH ( */
+   else if (Matches("COPY|\\copy", MatchAny, "TO",
MatchAnyExcept("PROGRAM"), "WITH", "(") ||
+            Matches("COPY|\\copy", MatchAny, "TO", "PROGRAM",
MatchAny, "WITH", "("))

Does it make sense to replace these two lines with the following one line?

else if (Matches("COPY|\\copy", MatchAny, "TO", MatchAny, MatchAnyN,
"WITH", "("))

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

On Thu, 25 Sep 2025 14:31:18 -0700
Masahiko Sawada <sawada.mshk@gmail.com> wrote:

I fixed it to use 'generator'.

LGTM. I've pushed the 0001 patch.

Thank you!

---
-   /* Complete COPY <sth> FROM <sth> */
-   else if (Matches("COPY|\\copy", MatchAny, "FROM", MatchAny))
+   /* Complete COPY <sth> FROM [PROGRAM] <sth> */
+   else if (Matches("COPY|\\copy", MatchAny, "FROM",
MatchAnyExcept("PROGRAM")) ||
+            Matches("COPY|\\copy", MatchAny, "FROM", "PROGRAM", MatchAny))

I see this kind of conversion many places in the patch; convert one
condition with MatchAny into two conditions with
MatchAnyExcept("PROGRAM") and '"PROGRAM", MatchAny'. How about
simplifying it using MatchAnyN. For example,

else if (Matches("COPY|\\copy", MatchAny, "FROM", MatchAny, MatchAnyN))

We could simplify this by using MatchAnyN, but doing so would cause "WITH ("
or "WHERE" to be suggested after "WITH (...)", even though that is not allowed
by the syntax. This could be misleading for users, so I wonder whether it is
worth adding a bit of complexity to prevent possible confusion.

There was a mistake in the previous statement: "WHERE" appearing after "WITH (...)"
is actually correct. However, this also results in "WITH" being suggested after
"WHERE", which is not permitted by the syntax.

True. How about other places? That is, where we check the completion
after "WITH (". For example:

-   /* Complete COPY <sth> TO filename WITH ( */
-   else if (Matches("COPY|\\copy", MatchAny, "TO", MatchAny, "WITH", "("))
+   /* Complete COPY <sth> TO [PROGRAM] filename WITH ( */
+   else if (Matches("COPY|\\copy", MatchAny, "TO",
MatchAnyExcept("PROGRAM"), "WITH", "(") ||
+            Matches("COPY|\\copy", MatchAny, "TO", "PROGRAM",
MatchAny, "WITH", "("))

Does it make sense to replace these two lines with the following one line?

else if (Matches("COPY|\\copy", MatchAny, "TO", MatchAny, MatchAnyN,
"WITH", "("))

That works for other places where options are suggested after "WITH (" and
"WHERE" is suggested after "WITH (*)".

I've attached updated patches using MatchAnyN following your suggestion.

The patch 0002 was also changed to use Matches, since MathAnyN cannot be used
with HeadMatches. I don't think this is a problem, because the COPY command cannot
be nested and "COPY or "\copy" would always appear at the beginning.

Regards,
Yugo Nagata

--
Yugo Nagata <nagata@sraoss.co.jp>

On Thu, Sep 25, 2025 at 6:16 PM Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Thu, 25 Sep 2025 14:31:18 -0700
Masahiko Sawada <sawada.mshk@gmail.com> wrote:

I fixed it to use 'generator'.

LGTM. I've pushed the 0001 patch.

Thank you!

---
-   /* Complete COPY <sth> FROM <sth> */
-   else if (Matches("COPY|\\copy", MatchAny, "FROM", MatchAny))
+   /* Complete COPY <sth> FROM [PROGRAM] <sth> */
+   else if (Matches("COPY|\\copy", MatchAny, "FROM",
MatchAnyExcept("PROGRAM")) ||
+            Matches("COPY|\\copy", MatchAny, "FROM", "PROGRAM", MatchAny))

I see this kind of conversion many places in the patch; convert one
condition with MatchAny into two conditions with
MatchAnyExcept("PROGRAM") and '"PROGRAM", MatchAny'. How about
simplifying it using MatchAnyN. For example,

else if (Matches("COPY|\\copy", MatchAny, "FROM", MatchAny, MatchAnyN))

We could simplify this by using MatchAnyN, but doing so would cause "WITH ("
or "WHERE" to be suggested after "WITH (...)", even though that is not allowed
by the syntax. This could be misleading for users, so I wonder whether it is
worth adding a bit of complexity to prevent possible confusion.

There was a mistake in the previous statement: "WHERE" appearing after "WITH (...)"
is actually correct. However, this also results in "WITH" being suggested after
"WHERE", which is not permitted by the syntax.

True. How about other places? That is, where we check the completion
after "WITH (". For example:

-   /* Complete COPY <sth> TO filename WITH ( */
-   else if (Matches("COPY|\\copy", MatchAny, "TO", MatchAny, "WITH", "("))
+   /* Complete COPY <sth> TO [PROGRAM] filename WITH ( */
+   else if (Matches("COPY|\\copy", MatchAny, "TO",
MatchAnyExcept("PROGRAM"), "WITH", "(") ||
+            Matches("COPY|\\copy", MatchAny, "TO", "PROGRAM",
MatchAny, "WITH", "("))

Does it make sense to replace these two lines with the following one line?

else if (Matches("COPY|\\copy", MatchAny, "TO", MatchAny, MatchAnyN,
"WITH", "("))

That works for other places where options are suggested after "WITH (" and
"WHERE" is suggested after "WITH (*)".

I've attached updated patches using MatchAnyN following your suggestion.

Thank you for updating the patch!

After reviewing both the v6 and v7 patches, I realize that your
original approach in v6 is actually better than what I suggested.
While it requires more lines of code, it provides more precise
completions. Additionally, since most of these extra lines are removed
by the next patch (v6-0003), the code size isn't really an issue.
Would it be possible to withdraw my previous comments and proceed with
the v6 approach? I apologize for the back-and-forth on this.

I have two review comments about the complete_from_files() function:

+ * This function wraps _complete_from_files() so that both literal keywords
+ * and filenames can be included in the completion results.
+ *
+ * If completion_charpp is set to a null-terminated array of literal keywords,
+ * those keywords are added to the completion results alongside filenames,
+ * as long as they case-insensitively match the current input.

How about rephrasing the comments to the following?

/*
* This function returns in order one of a fixed, NULL pointer terminated list
* of string that matches file names or optionally specified list of keywords.
*
* If completion_charpp is set to a null-terminated array of literal keywords,
* those keywords are added to the completion results alongside filenames if
* they case-insensitively match the current input.
*/

---
+   /* Return a filename that matches */
+   if (!files_done && (result = _complete_from_files(text, state)))
+       return result;
+   else if (!completion_charpp)
+       return NULL;
+   else
+       files_done = true;

It works but it's odd a bit that we don't set files_done to true if
completion_charpp is not NULL. I think it becomes more readable if we
could set files_done to true if _complete_from_files() doesn't return
a string and proceed with the hard-wired keywords.

The attached patch that can be applied on top of v6-0002 patch,
implements my suggestions and includes pgindent fixes. Please review
these changes.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

On Fri, Oct 3, 2025 at 2:06 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Thu, Sep 25, 2025 at 6:16 PM Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Thu, 25 Sep 2025 14:31:18 -0700
Masahiko Sawada <sawada.mshk@gmail.com> wrote:

I fixed it to use 'generator'.

LGTM. I've pushed the 0001 patch.

Thank you!

---
-   /* Complete COPY <sth> FROM <sth> */
-   else if (Matches("COPY|\\copy", MatchAny, "FROM", MatchAny))
+   /* Complete COPY <sth> FROM [PROGRAM] <sth> */
+   else if (Matches("COPY|\\copy", MatchAny, "FROM",
MatchAnyExcept("PROGRAM")) ||
+            Matches("COPY|\\copy", MatchAny, "FROM", "PROGRAM", MatchAny))

I see this kind of conversion many places in the patch; convert one
condition with MatchAny into two conditions with
MatchAnyExcept("PROGRAM") and '"PROGRAM", MatchAny'. How about
simplifying it using MatchAnyN. For example,

else if (Matches("COPY|\\copy", MatchAny, "FROM", MatchAny, MatchAnyN))

We could simplify this by using MatchAnyN, but doing so would cause "WITH ("
or "WHERE" to be suggested after "WITH (...)", even though that is not allowed
by the syntax. This could be misleading for users, so I wonder whether it is
worth adding a bit of complexity to prevent possible confusion.

There was a mistake in the previous statement: "WHERE" appearing after "WITH (...)"
is actually correct. However, this also results in "WITH" being suggested after
"WHERE", which is not permitted by the syntax.

True. How about other places? That is, where we check the completion
after "WITH (". For example:

-   /* Complete COPY <sth> TO filename WITH ( */
-   else if (Matches("COPY|\\copy", MatchAny, "TO", MatchAny, "WITH", "("))
+   /* Complete COPY <sth> TO [PROGRAM] filename WITH ( */
+   else if (Matches("COPY|\\copy", MatchAny, "TO",
MatchAnyExcept("PROGRAM"), "WITH", "(") ||
+            Matches("COPY|\\copy", MatchAny, "TO", "PROGRAM",
MatchAny, "WITH", "("))

Does it make sense to replace these two lines with the following one line?

else if (Matches("COPY|\\copy", MatchAny, "TO", MatchAny, MatchAnyN,
"WITH", "("))

That works for other places where options are suggested after "WITH (" and
"WHERE" is suggested after "WITH (*)".

I've attached updated patches using MatchAnyN following your suggestion.

Thank you for updating the patch!

After reviewing both the v6 and v7 patches, I realize that your
original approach in v6 is actually better than what I suggested.
While it requires more lines of code, it provides more precise
completions. Additionally, since most of these extra lines are removed
by the next patch (v6-0003), the code size isn't really an issue.
Would it be possible to withdraw my previous comments and proceed with
the v6 approach? I apologize for the back-and-forth on this.

I have two review comments about the complete_from_files() function:

+ * This function wraps _complete_from_files() so that both literal keywords
+ * and filenames can be included in the completion results.
+ *
+ * If completion_charpp is set to a null-terminated array of literal keywords,
+ * those keywords are added to the completion results alongside filenames,
+ * as long as they case-insensitively match the current input.

How about rephrasing the comments to the following?

/*
* This function returns in order one of a fixed, NULL pointer terminated list
* of string that matches file names or optionally specified list of keywords.
*
* If completion_charpp is set to a null-terminated array of literal keywords,
* those keywords are added to the completion results alongside filenames if
* they case-insensitively match the current input.
*/

---
+   /* Return a filename that matches */
+   if (!files_done && (result = _complete_from_files(text, state)))
+       return result;
+   else if (!completion_charpp)
+       return NULL;
+   else
+       files_done = true;

It works but it's odd a bit that we don't set files_done to true if
completion_charpp is not NULL. I think it becomes more readable if we
could set files_done to true if _complete_from_files() doesn't return
a string and proceed with the hard-wired keywords.

The attached patch that can be applied on top of v6-0002 patch,
implements my suggestions and includes pgindent fixes. Please review
these changes.

I believe we can split the first patch into two patches: one adds
support for STDIN/STDOUT with COMPLETE_WIHT_FILES_PLUS() and another
one adds support for COPY syntaxes using the PROGRAM clause. I've
attached the reorganized patch set and made cosmetic changes to the
0003 patch (i.e., improving COPY option list). What do you think?

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

On Mon, Oct 6, 2025 at 5:03 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Fri, Oct 3, 2025 at 2:06 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Thu, Sep 25, 2025 at 6:16 PM Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Thu, 25 Sep 2025 14:31:18 -0700
Masahiko Sawada <sawada.mshk@gmail.com> wrote:

I fixed it to use 'generator'.

LGTM. I've pushed the 0001 patch.

Thank you!

---
-   /* Complete COPY <sth> FROM <sth> */
-   else if (Matches("COPY|\\copy", MatchAny, "FROM", MatchAny))
+   /* Complete COPY <sth> FROM [PROGRAM] <sth> */
+   else if (Matches("COPY|\\copy", MatchAny, "FROM",
MatchAnyExcept("PROGRAM")) ||
+            Matches("COPY|\\copy", MatchAny, "FROM", "PROGRAM", MatchAny))

I see this kind of conversion many places in the patch; convert one
condition with MatchAny into two conditions with
MatchAnyExcept("PROGRAM") and '"PROGRAM", MatchAny'. How about
simplifying it using MatchAnyN. For example,

else if (Matches("COPY|\\copy", MatchAny, "FROM", MatchAny, MatchAnyN))

We could simplify this by using MatchAnyN, but doing so would cause "WITH ("
or "WHERE" to be suggested after "WITH (...)", even though that is not allowed
by the syntax. This could be misleading for users, so I wonder whether it is
worth adding a bit of complexity to prevent possible confusion.

There was a mistake in the previous statement: "WHERE" appearing after "WITH (...)"
is actually correct. However, this also results in "WITH" being suggested after
"WHERE", which is not permitted by the syntax.

True. How about other places? That is, where we check the completion
after "WITH (". For example:

-   /* Complete COPY <sth> TO filename WITH ( */
-   else if (Matches("COPY|\\copy", MatchAny, "TO", MatchAny, "WITH", "("))
+   /* Complete COPY <sth> TO [PROGRAM] filename WITH ( */
+   else if (Matches("COPY|\\copy", MatchAny, "TO",
MatchAnyExcept("PROGRAM"), "WITH", "(") ||
+            Matches("COPY|\\copy", MatchAny, "TO", "PROGRAM",
MatchAny, "WITH", "("))

Does it make sense to replace these two lines with the following one line?

else if (Matches("COPY|\\copy", MatchAny, "TO", MatchAny, MatchAnyN,
"WITH", "("))

That works for other places where options are suggested after "WITH (" and
"WHERE" is suggested after "WITH (*)".

I've attached updated patches using MatchAnyN following your suggestion.

Thank you for updating the patch!

After reviewing both the v6 and v7 patches, I realize that your
original approach in v6 is actually better than what I suggested.
While it requires more lines of code, it provides more precise
completions. Additionally, since most of these extra lines are removed
by the next patch (v6-0003), the code size isn't really an issue.
Would it be possible to withdraw my previous comments and proceed with
the v6 approach? I apologize for the back-and-forth on this.

I have two review comments about the complete_from_files() function:

+ * This function wraps _complete_from_files() so that both literal keywords
+ * and filenames can be included in the completion results.
+ *
+ * If completion_charpp is set to a null-terminated array of literal keywords,
+ * those keywords are added to the completion results alongside filenames,
+ * as long as they case-insensitively match the current input.

How about rephrasing the comments to the following?

/*
* This function returns in order one of a fixed, NULL pointer terminated list
* of string that matches file names or optionally specified list of keywords.
*
* If completion_charpp is set to a null-terminated array of literal keywords,
* those keywords are added to the completion results alongside filenames if
* they case-insensitively match the current input.
*/

---
+   /* Return a filename that matches */
+   if (!files_done && (result = _complete_from_files(text, state)))
+       return result;
+   else if (!completion_charpp)
+       return NULL;
+   else
+       files_done = true;

It works but it's odd a bit that we don't set files_done to true if
completion_charpp is not NULL. I think it becomes more readable if we
could set files_done to true if _complete_from_files() doesn't return
a string and proceed with the hard-wired keywords.

The attached patch that can be applied on top of v6-0002 patch,
implements my suggestions and includes pgindent fixes. Please review
these changes.

I believe we can split the first patch into two patches: one adds
support for STDIN/STDOUT with COMPLETE_WIHT_FILES_PLUS() and another
one adds support for COPY syntaxes using the PROGRAM clause. I've
attached the reorganized patch set and made cosmetic changes to the
0003 patch (i.e., improving COPY option list). What do you think?

Pushed the first two patches.

As for the remaining patch that adds tab completion for COPY option
lists, I think it would be a good idea to add tab completion for other
options too such as HEADER and FREEZE.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

Hi Sawada-san,

I still cannot confirm that my previous email has been archived [1]https://www.postgresql.org/list/pgsql-hackers/since/202511130000/,
so let me resend it.
I apologize again for the repeated emails.

[1]: https://www.postgresql.org/list/pgsql-hackers/since/202511130000/

On Tue, 4 Nov 2025 10:57:14 -0800
Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Mon, Oct 6, 2025 at 5:03 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Fri, Oct 3, 2025 at 2:06 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Thu, Sep 25, 2025 at 6:16 PM Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Thu, 25 Sep 2025 14:31:18 -0700
Masahiko Sawada <sawada.mshk@gmail.com> wrote:

I fixed it to use 'generator'.

LGTM. I've pushed the 0001 patch.

Thank you!

---
-   /* Complete COPY <sth> FROM <sth> */
-   else if (Matches("COPY|\\copy", MatchAny, "FROM", MatchAny))
+   /* Complete COPY <sth> FROM [PROGRAM] <sth> */
+   else if (Matches("COPY|\\copy", MatchAny, "FROM",
MatchAnyExcept("PROGRAM")) ||
+            Matches("COPY|\\copy", MatchAny, "FROM", "PROGRAM", MatchAny))

I see this kind of conversion many places in the patch; convert one
condition with MatchAny into two conditions with
MatchAnyExcept("PROGRAM") and '"PROGRAM", MatchAny'. How about
simplifying it using MatchAnyN. For example,

else if (Matches("COPY|\\copy", MatchAny, "FROM", MatchAny, MatchAnyN))

We could simplify this by using MatchAnyN, but doing so would cause "WITH ("
or "WHERE" to be suggested after "WITH (...)", even though that is not allowed
by the syntax. This could be misleading for users, so I wonder whether it is
worth adding a bit of complexity to prevent possible confusion.

There was a mistake in the previous statement: "WHERE" appearing after "WITH (...)"
is actually correct. However, this also results in "WITH" being suggested after
"WHERE", which is not permitted by the syntax.

True. How about other places? That is, where we check the completion
after "WITH (". For example:

-   /* Complete COPY <sth> TO filename WITH ( */
-   else if (Matches("COPY|\\copy", MatchAny, "TO", MatchAny, "WITH", "("))
+   /* Complete COPY <sth> TO [PROGRAM] filename WITH ( */
+   else if (Matches("COPY|\\copy", MatchAny, "TO",
MatchAnyExcept("PROGRAM"), "WITH", "(") ||
+            Matches("COPY|\\copy", MatchAny, "TO", "PROGRAM",
MatchAny, "WITH", "("))

Does it make sense to replace these two lines with the following one line?

else if (Matches("COPY|\\copy", MatchAny, "TO", MatchAny, MatchAnyN,
"WITH", "("))

That works for other places where options are suggested after "WITH (" and
"WHERE" is suggested after "WITH (*)".

I've attached updated patches using MatchAnyN following your suggestion.

Thank you for updating the patch!

After reviewing both the v6 and v7 patches, I realize that your
original approach in v6 is actually better than what I suggested.
While it requires more lines of code, it provides more precise
completions. Additionally, since most of these extra lines are removed
by the next patch (v6-0003), the code size isn't really an issue.
Would it be possible to withdraw my previous comments and proceed with
the v6 approach? I apologize for the back-and-forth on this.

I have two review comments about the complete_from_files() function:

+ * This function wraps _complete_from_files() so that both literal keywords
+ * and filenames can be included in the completion results.
+ *
+ * If completion_charpp is set to a null-terminated array of literal keywords,
+ * those keywords are added to the completion results alongside filenames,
+ * as long as they case-insensitively match the current input.

How about rephrasing the comments to the following?

/*
* This function returns in order one of a fixed, NULL pointer terminated list
* of string that matches file names or optionally specified list of keywords.
*
* If completion_charpp is set to a null-terminated array of literal keywords,
* those keywords are added to the completion results alongside filenames if
* they case-insensitively match the current input.
*/

---
+   /* Return a filename that matches */
+   if (!files_done && (result = _complete_from_files(text, state)))
+       return result;
+   else if (!completion_charpp)
+       return NULL;
+   else
+       files_done = true;

It works but it's odd a bit that we don't set files_done to true if
completion_charpp is not NULL. I think it becomes more readable if we
could set files_done to true if _complete_from_files() doesn't return
a string and proceed with the hard-wired keywords.

The attached patch that can be applied on top of v6-0002 patch,
implements my suggestions and includes pgindent fixes. Please review
these changes.

I believe we can split the first patch into two patches: one adds
support for STDIN/STDOUT with COMPLETE_WIHT_FILES_PLUS() and another
one adds support for COPY syntaxes using the PROGRAM clause. I've
attached the reorganized patch set and made cosmetic changes to the
0003 patch (i.e., improving COPY option list). What do you think?

Pushed the first two patches.

As for the remaining patch that adds tab completion for COPY option
lists, I think it would be a good idea to add tab completion for other
options too such as HEADER and FREEZE.

I'm sorry for the late reply.
Thank you for reviewing and committing the patches!

I've attached an updated patch that includes completions for the HEADER
and FREEZE option values.

Regards,
Yugo Nagata

--
Yugo Nagata <nagata@sraoss.co.jp>

On Thu, Nov 13, 2025 at 11:55 PM Yugo Nagata <nagata@sraoss.co.jp> wrote:

Hi Sawada-san,

I still cannot confirm that my previous email has been archived [1],
so let me resend it.
I apologize again for the repeated emails.

[1] https://www.postgresql.org/list/pgsql-hackers/since/202511130000/

On Tue, 4 Nov 2025 10:57:14 -0800
Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Mon, Oct 6, 2025 at 5:03 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Fri, Oct 3, 2025 at 2:06 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Thu, Sep 25, 2025 at 6:16 PM Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Thu, 25 Sep 2025 14:31:18 -0700
Masahiko Sawada <sawada.mshk@gmail.com> wrote:

I fixed it to use 'generator'.

LGTM. I've pushed the 0001 patch.

Thank you!

---
-   /* Complete COPY <sth> FROM <sth> */
-   else if (Matches("COPY|\\copy", MatchAny, "FROM", MatchAny))
+   /* Complete COPY <sth> FROM [PROGRAM] <sth> */
+   else if (Matches("COPY|\\copy", MatchAny, "FROM",
MatchAnyExcept("PROGRAM")) ||
+            Matches("COPY|\\copy", MatchAny, "FROM", "PROGRAM", MatchAny))

I see this kind of conversion many places in the patch; convert one
condition with MatchAny into two conditions with
MatchAnyExcept("PROGRAM") and '"PROGRAM", MatchAny'. How about
simplifying it using MatchAnyN. For example,

else if (Matches("COPY|\\copy", MatchAny, "FROM", MatchAny, MatchAnyN))

We could simplify this by using MatchAnyN, but doing so would cause "WITH ("
or "WHERE" to be suggested after "WITH (...)", even though that is not allowed
by the syntax. This could be misleading for users, so I wonder whether it is
worth adding a bit of complexity to prevent possible confusion.

There was a mistake in the previous statement: "WHERE" appearing after "WITH (...)"
is actually correct. However, this also results in "WITH" being suggested after
"WHERE", which is not permitted by the syntax.

True. How about other places? That is, where we check the completion
after "WITH (". For example:

-   /* Complete COPY <sth> TO filename WITH ( */
-   else if (Matches("COPY|\\copy", MatchAny, "TO", MatchAny, "WITH", "("))
+   /* Complete COPY <sth> TO [PROGRAM] filename WITH ( */
+   else if (Matches("COPY|\\copy", MatchAny, "TO",
MatchAnyExcept("PROGRAM"), "WITH", "(") ||
+            Matches("COPY|\\copy", MatchAny, "TO", "PROGRAM",
MatchAny, "WITH", "("))

Does it make sense to replace these two lines with the following one line?

else if (Matches("COPY|\\copy", MatchAny, "TO", MatchAny, MatchAnyN,
"WITH", "("))

That works for other places where options are suggested after "WITH (" and
"WHERE" is suggested after "WITH (*)".

I've attached updated patches using MatchAnyN following your suggestion.

Thank you for updating the patch!

After reviewing both the v6 and v7 patches, I realize that your
original approach in v6 is actually better than what I suggested.
While it requires more lines of code, it provides more precise
completions. Additionally, since most of these extra lines are removed
by the next patch (v6-0003), the code size isn't really an issue.
Would it be possible to withdraw my previous comments and proceed with
the v6 approach? I apologize for the back-and-forth on this.

I have two review comments about the complete_from_files() function:

+ * This function wraps _complete_from_files() so that both literal keywords
+ * and filenames can be included in the completion results.
+ *
+ * If completion_charpp is set to a null-terminated array of literal keywords,
+ * those keywords are added to the completion results alongside filenames,
+ * as long as they case-insensitively match the current input.

How about rephrasing the comments to the following?

/*
* This function returns in order one of a fixed, NULL pointer terminated list
* of string that matches file names or optionally specified list of keywords.
*
* If completion_charpp is set to a null-terminated array of literal keywords,
* those keywords are added to the completion results alongside filenames if
* they case-insensitively match the current input.
*/

---
+   /* Return a filename that matches */
+   if (!files_done && (result = _complete_from_files(text, state)))
+       return result;
+   else if (!completion_charpp)
+       return NULL;
+   else
+       files_done = true;

It works but it's odd a bit that we don't set files_done to true if
completion_charpp is not NULL. I think it becomes more readable if we
could set files_done to true if _complete_from_files() doesn't return
a string and proceed with the hard-wired keywords.

The attached patch that can be applied on top of v6-0002 patch,
implements my suggestions and includes pgindent fixes. Please review
these changes.

I believe we can split the first patch into two patches: one adds
support for STDIN/STDOUT with COMPLETE_WIHT_FILES_PLUS() and another
one adds support for COPY syntaxes using the PROGRAM clause. I've
attached the reorganized patch set and made cosmetic changes to the
0003 patch (i.e., improving COPY option list). What do you think?

Pushed the first two patches.

As for the remaining patch that adds tab completion for COPY option
lists, I think it would be a good idea to add tab completion for other
options too such as HEADER and FREEZE.

I'm sorry for the late reply.
Thank you for reviewing and committing the patches!

I've attached an updated patch that includes completions for the HEADER
and FREEZE option values.

Thank you for updating the patch! I have one question about the
following change:

+   /* Complete COPY <sth> FROM|TO [PROGRAM] filename WITH ( */
+   else if (HeadMatches("COPY|\\copy", MatchAny, "FROM|TO",
MatchAnyExcept("PROGRAM"), "WITH", "(") ||
+            HeadMatches("COPY|\\copy", MatchAny, "FROM|TO",
"PROGRAM", MatchAny, "WITH", "("))
+   {
+       if (!HeadMatches("COPY|\\copy", MatchAny, "FROM|TO",
MatchAnyExcept("PROGRAM"), "WITH", "(*)") &&
+           !HeadMatches("COPY|\\copy", MatchAny, "FROM|TO",
"PROGRAM", MatchAny, "WITH", "(*)"))
+       {

Given get_previous_words() treats a completed parenthesized option
list as one word, why is the later if statement necessary? In the
situation where the condition of the later if statement becomes false,
the condition of the first 'else if' statement should have become
false, no?

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

On Mon, 24 Nov 2025 15:32:36 -0800
Masahiko Sawada <sawada.mshk@gmail.com> wrote:

I'm sorry for the late response.

Thank you for updating the patch! I have one question about the
following change:

+   /* Complete COPY <sth> FROM|TO [PROGRAM] filename WITH ( */
+   else if (HeadMatches("COPY|\\copy", MatchAny, "FROM|TO",
MatchAnyExcept("PROGRAM"), "WITH", "(") ||
+            HeadMatches("COPY|\\copy", MatchAny, "FROM|TO",
"PROGRAM", MatchAny, "WITH", "("))
+   {
+       if (!HeadMatches("COPY|\\copy", MatchAny, "FROM|TO",
MatchAnyExcept("PROGRAM"), "WITH", "(*)") &&
+           !HeadMatches("COPY|\\copy", MatchAny, "FROM|TO",
"PROGRAM", MatchAny, "WITH", "(*)"))
+       {

Given get_previous_words() treats a completed parenthesized option
list as one word, why is the later if statement necessary? In the
situation where the condition of the later if statement becomes false,
the condition of the first 'else if' statement should have become
false, no?

First of all, the condition of the outer 'else if' was wrong.
What I actually meant was:

 +   /* Complete COPY <sth> FROM|TO [PROGRAM] filename WITH ( */
 +   else if (HeadMatches("COPY|\\copy", MatchAny, "FROM|TO",
MatchAnyExcept("PROGRAM"), "WITH", "(*") ||
 +            HeadMatches("COPY|\\copy", MatchAny, "FROM|TO",
 "PROGRAM", MatchAny, "WITH", "(*"))

That is, the last word should be "(*" instead of "(".
I've attached an updated patch reflecting this fix.

This patterm is mimic how option lists are handled in the other places.
For example:

3160 else if (HeadMatches("ANALYZE", "(*") &&
3161 !HeadMatches("ANALYZE", "(*)"))

This prevents option completion after a completed paranthesized option list.
For example, after "ANALYZE (VERVOSE ON) tbl (", no ANALYZE options are suggested,
instead column names of the table "tbl" are suggested.

Similarly, this change prevents completion of COPY options after
"COPY tbl FROM stdin WITH ( FORMAT text ) WHERE (", for example.

Regards,
Yugo Nagata

--
Yugo Nagata <nagata@sraoss.co.jp>

On Tue, 23 Dec 2025 00:37:05 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Mon, 24 Nov 2025 15:32:36 -0800
Masahiko Sawada <sawada.mshk@gmail.com> wrote:

I'm sorry for the late response.

Thank you for updating the patch! I have one question about the
following change:

+   /* Complete COPY <sth> FROM|TO [PROGRAM] filename WITH ( */
+   else if (HeadMatches("COPY|\\copy", MatchAny, "FROM|TO",
MatchAnyExcept("PROGRAM"), "WITH", "(") ||
+            HeadMatches("COPY|\\copy", MatchAny, "FROM|TO",
"PROGRAM", MatchAny, "WITH", "("))
+   {
+       if (!HeadMatches("COPY|\\copy", MatchAny, "FROM|TO",
MatchAnyExcept("PROGRAM"), "WITH", "(*)") &&
+           !HeadMatches("COPY|\\copy", MatchAny, "FROM|TO",
"PROGRAM", MatchAny, "WITH", "(*)"))
+       {

Given get_previous_words() treats a completed parenthesized option
list as one word, why is the later if statement necessary? In the
situation where the condition of the later if statement becomes false,
the condition of the first 'else if' statement should have become
false, no?

First of all, the condition of the outer 'else if' was wrong.
What I actually meant was:

+   /* Complete COPY <sth> FROM|TO [PROGRAM] filename WITH ( */
+   else if (HeadMatches("COPY|\\copy", MatchAny, "FROM|TO",
MatchAnyExcept("PROGRAM"), "WITH", "(*") ||
+            HeadMatches("COPY|\\copy", MatchAny, "FROM|TO",
"PROGRAM", MatchAny, "WITH", "(*"))

That is, the last word should be "(*" instead of "(".
I've attached an updated patch reflecting this fix.

This patterm is mimic how option lists are handled in the other places.
For example:

3160 else if (HeadMatches("ANALYZE", "(*") &&
3161 !HeadMatches("ANALYZE", "(*)"))

This prevents option completion after a completed paranthesized option list.
For example, after "ANALYZE (VERVOSE ON) tbl (", no ANALYZE options are suggested,
instead column names of the table "tbl" are suggested.

Similarly, this change prevents completion of COPY options after
"COPY tbl FROM stdin WITH ( FORMAT text ) WHERE (", for example.

By the way, while investigating your comments, I noticed an issue with
tab completion for VACUUM option values. I reported it in a separate
thread [1]/messages/by-id/20251223021509.19bba68ecbbc70c9f983c2b4@sraoss.co.jp, since it appears to be a bug introduced by another commit.

[1]: /messages/by-id/20251223021509.19bba68ecbbc70c9f983c2b4@sraoss.co.jp

--
Yugo Nagata <nagata@sraoss.co.jp>