Re: [PATCH] COPY vs \copy HINT

On 2 September 2016 at 04:28, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Craig Ringer <craig(at)2ndquadrant(dot)com> writes:
>> On 12 August 2016 at 16:34, Christoph Berg <myon(at)debian(dot)org> wrote:
>>> Also, I vaguely get what you wanted to say with "a driver ...
>>> wrapper", but it's pretty nonsensical if one doesn't know about the
>>> protocol details. I don't have a better suggestion now, but I think it
>>> needs rephrasing.
>
>> I don't like it either, but didn't come up with anything better. The
>> problem is that every driver calls it something different.
>
> A few thoughts on this patch:

Thanks for the review. I'll leave the first change pending your
comments, the others are made, though I'm not completely sure we
should restrict it to ENOENT.

> 1. I don't really think the HINT is appropriate for the not-absolute-path
> case.

Why? If the user runs

# COPY sometable FROM 'localfile.csv' WITH (FORMAT CSV);
ERROR: could not open file "localfile.csv" for reading: No such file
or directory

they're going to be just as confused by this error as by:

# COPY batch_demo FROM '/tmp/localfile.csv' WITH (FORMAT CSV);
ERROR: could not open file "/tmp/localfile.csv" for reading: No such
file or directory

so I don't really see the rationale for this change.

> 2. I don't think it's appropriate for all possible cases of AllocateFile
> failure either, eg surely not for EACCES or similar situations where we
> did find a file. Maybe print it only for ENOENT? (See for example
> report_newlocale_failure() for technique.)

I thought about that but figured it didn't really matter too much,
when thinking about examples like

# COPY batch_demo FROM '/root/secret.csv' WITH (FORMAT CSV);
ERROR: could not open file "/root/secret.csv" for reading: Permission denied

or whatever, where the user doesn't understand why they can't read the
file given that their local client has permission to do so.

I don't feel strongly about this and think that the error on ENOENT is
by far the most important, so I'll adjust it per your recommendation.

> 3. As for the wording, maybe you could do it like this:
>
> HINT: COPY copies to[from] a file on the PostgreSQL server, not on the
> client. You may want a client-side facility such as psql's \copy.
>
> That avoids trying to invent a name for other implementations.

I like that wording a lot more, thanks. Adopted.

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services