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>
