Re: COPY (... tab completion

Hi,

Here is a patch which adds tab completion for COPY with a query.
Currently there is only completion for COPY with a relation.

Andreas

Hi,

I have an updated patch which uses Matches() rather than TailMatches().

Andreas

I think this would be a useful addition. A couple of problems:

This change in the comment doesn't make sense to me and doesn't seem to
match the code:

- /* If we have COPY [BINARY] <sth>, complete it with "TO" or "FROM" */
+ /* If we have COPY|BINARY <sth>, complete it with "TO" or "FROM" */

The list of commands to allow as the "query" inside the parentheses is
documented to be: SELECT, VALUES, INSERT, UPDATE or DELETE; and actually
TABLE should also work. Your list doesn't include all of those. So
please adjust that.

On 1/10/16 8:01 PM, Peter Eisentraut wrote:
> The list of commands to allow as the "query" inside the parentheses is
> documented to be: SELECT, VALUES, INSERT, UPDATE or DELETE; and actually
> TABLE should also work. Your list doesn't include all of those.

To be fair, this is actually a recent new feature.

Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> I think this would be a useful addition. A couple of problems:
> This change in the comment doesn't make sense to me and doesn't seem to
> match the code:

> - /* If we have COPY [BINARY] <sth>, complete it with "TO" or "FROM" */
> + /* If we have COPY|BINARY <sth>, complete it with "TO" or "FROM" */

Offhand, that looks like an accidental reversion of a change made in
9b181b0363deb65b15a9feaf3eb74f86707498a9.

regards, tom lane

On 01/11/2016 02:01 AM, Peter Eisentraut wrote:
> I think this would be a useful addition. A couple of problems:

Thanks for the review. A new version is attached.

> This change in the comment doesn't make sense to me and doesn't seem to
> match the code:
>
> - /* If we have COPY [BINARY] <sth>, complete it with "TO" or "FROM" */
> + /* If we have COPY|BINARY <sth>, complete it with "TO" or "FROM" */

Fixed. As Tom correctly guessed this was the result of a mistake when
rebasing.

> The list of commands to allow as the "query" inside the parentheses is
> documented to be: SELECT, VALUES, INSERT, UPDATE or DELETE; and actually
> TABLE should also work. Your list doesn't include all of those. So
> please adjust that.

Fixed. And TABLE works too.

Andreas

On 01/19/2016 01:57 AM, Andreas Karlsson wrote:
> Thanks for the review. A new version is attached.

Whops, attached the wrong file.

Andreas

On Tue, Jan 19, 2016 at 10:12 AM, Andreas Karlsson <andreas(at)proxel(dot)se> wrote:
> On 01/19/2016 01:57 AM, Andreas Karlsson wrote:
>>
>> Thanks for the review. A new version is attached.
>
>
> Whops, attached the wrong file.

+ /* If we have COPY BINARY, compelete with list of tables */
s/compelete/complete

+ else if (TailMatches2("COPY|\\copy", "("))
+ COMPLETE_WITH_LIST7("SELECT", "TABLE", "VALUES", "INSERT",
"UPDATE", "DELETE", "WITH");
This one should be Matches, no?
--
Michael

On 01/19/2016 07:55 AM, Michael Paquier wrote:
> + /* If we have COPY BINARY, compelete with list of tables */
> s/compelete/complete

Fixed.

> + else if (TailMatches2("COPY|\\copy", "("))
> + COMPLETE_WITH_LIST7("SELECT", "TABLE", "VALUES", "INSERT",
> "UPDATE", "DELETE", "WITH");
> This one should be Matches, no?

Yep, fixed.

Andreas

On Tue, Jan 19, 2016 at 8:00 PM, Andreas Karlsson <andreas(at)proxel(dot)se> wrote:
> On 01/19/2016 07:55 AM, Michael Paquier wrote:
>>
>> + /* If we have COPY BINARY, compelete with list of tables */
>> s/compelete/complete
>
> Fixed.
>
>> + else if (TailMatches2("COPY|\\copy", "("))
>> + COMPLETE_WITH_LIST7("SELECT", "TABLE", "VALUES", "INSERT",
>> "UPDATE", "DELETE", "WITH");
>> This one should be Matches, no?
>
> Yep, fixed.

Marked as ready for committer.

This patch makes me wonder: are we going to nuke the grammar "COPY [
BINARY ] table_name" at some point? This was used up to 7.3.
--
Michael

Michael Paquier <michael(dot)paquier(at)gmail(dot)com> writes:
> This patch makes me wonder: are we going to nuke the grammar "COPY [
> BINARY ] table_name" at some point? This was used up to 7.3.

I'm not particularly in a hurry to remove obsolete syntaxes, as long as
they're not blocking forward progress in some way. However, it seems
to me that it would certainly make sense to remove tab-completion support
for long-deprecated syntax.

regards, tom lane

On 1/19/16 6:00 AM, Andreas Karlsson wrote:
> On 01/19/2016 07:55 AM, Michael Paquier wrote:
>> + /* If we have COPY BINARY, compelete with list of tables */
>> s/compelete/complete
>
> Fixed.
>
>> + else if (TailMatches2("COPY|\\copy", "("))
>> + COMPLETE_WITH_LIST7("SELECT", "TABLE", "VALUES", "INSERT",
>> "UPDATE", "DELETE", "WITH");
>> This one should be Matches, no?
>
> Yep, fixed.

Committed v4, thanks.