Re: Added tab completion for the missing options in copy statement

On Sat, Jul 18, 2020 at 8:08 AM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Fri, Jul 17, 2020 at 05:28:51PM +0530, vignesh C wrote:
> > On Fri, Jul 17, 2020 at 11:15 AM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
> >> Not completely actually. The page of psql for \copy does not mention
> >> the optional where clause, and I think that it would be better to add
> >> that for consistency (perhaps that's the point raised by Ahsan?). I
> >> don't see much point in splitting the description of the meta-command
> >> into two lines as we already mix stdin and stdout for example which
> >> only apply to respectively "FROM" and "TO", so let's just append the
> >> conditional where clause at its end. Attached is a patch doing so
> >> that I intend to back-patch down to v12.
> >
> > I would like to split into 2 lines similar to documentation of
> > sql-copy which gives better readability, attaching a new patch in
> > similar lines.
>
> Fine by me. I have applied and back-patched this part down to 12.

Thanks for pushing the patch.

>
> >> Coming back to your proposal, another thing is that with your patch
> >> you recommend a syntax still present for compatibility reasons, but I
> >> don't think that we should recommend it to the users anymore, giving
> >> priority to the new grammar of the post-9.0 era. I would actually go
> >> as far as removing BINARY from the completion when specified just
> >> after COPY to simplify the code, and specify the list of available
> >> options after typing "COPY ... WITH (FORMAT ", with "text", "csv" and
> >> "binary". Adding completion for WHERE after COPY FROM is of course a
> >> good idea.
> >
> > I agree with your comments, and have made a new patch accordingly.
> > Thoughts?
>
> Nope, that's not what I meant. My point was to drop completely from
> the completion the past grammar we are keeping around for
> compatibility reasons, and just complete with the new grammar
> documented at the top of the COPY page. This leads me to the
> attached, which actually simplifies the code, adds more completion
> patterns with the mixes of WHERE/WITH depending on if FROM or TO is
> used, and at the end is less bug-prone if the grammar gets more
> extended. I have also added some completion for "WITH (FORMAT" for
> text, csv and binary.

This version of patch looks good, patch applies, make check & make
check-world passes.
This is not part of the new changes, this change already exists, I had
one small clarification on the below code:
/* Complete COPY ( with legal query commands */
else if (Matches("COPY|\\copy", "("))
COMPLETE_WITH("SELECT", "TABLE", "VALUES", "INSERT", "UPDATE",
"DELETE", "WITH");

Can we specify Insert/Update or delete with copy?
When I tried few scenarios I was getting the following error:
ERROR: COPY query must have a RETURNING clause

I might be missing some scenarios, just wanted to confirm if this is
kept intentionally.

Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com