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

From: Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp>
To: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
Cc: Pgsql Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: psql: tab-completion support for COPY ... TO/FROM STDIN, STDOUT, and PROGRAM
Date: 2025-09-26 01:16:35
Message-ID: 20250926101635.dea7cec62b02d1c614103f78@sraoss.co.jp
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, 25 Sep 2025 14:31:18 -0700
Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)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(at)sraoss(dot)co(dot)jp>

Attachment Content-Type Size
v7-0002-Improve-tab-completion-for-COPY-option-lists.patch text/x-diff 3.0 KB
v7-0001-Add-tab-completion-support-for-COPY-.-TO-FROM-STD.patch text/x-diff 7.7 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2025-09-26 01:20:19 Re: [BUG] temporary file usage report with extended protocol and unnamed portals
Previous Message Sami Imseih 2025-09-26 00:47:48 Re: Add support for entry counting in pgstats