Re: Review : Add hooks for pre- and post-processor executables for COPY and \copy

On 27.02.2013 11:38, Etsuro Fujita wrote:
> Agreed. ISTM that the comment on parse_slash_copy() needs to be changed:
>
> * table name can be double-quoted and can have a schema part.
> * column names can be double-quoted.
> * filename can be single-quoted like SQL literals.
> * command can be single-quoted like SQL literals.
>
> The last line must be:
>
> * command must be single-quoted like SQL literals.

Fixed that.

>>> Attached is a new version of this patch that I almost committed, but
>>> one thing caught my eye at the last minute: The error reporting is not
>>> very user-friendly. If the program fails after reading/writing some
>>> rows, the reason is printed to the log, but not the user:
>>>
>>> postgres=# copy foo to program '/tmp/midway-crash';
>>> ERROR: could not execute command "/tmp/midway-crash"
>>>
>>> the log has more details:
>>>
>>> LOG: child process exited with exit code 10
>>> STATEMENT: copy foo to program '/tmp/midway-crash';
>>> ERROR: could not execute command "/tmp/midway-crash"
>>> STATEMENT: copy foo to program '/tmp/midway-crash';
>>>
>>> I think we should arrange for the "child process exited with exit code
>>> 10" to be printed as errdetail to the client. Similarly, with psql
>>> \copy command, the "child process exited with exit code 10" command
>>> shouldn't be printed straight to stderr, but should go through
>>> psql_error.
>>>
>>> I'll try to refactor pclose_check tomorrow to do that. Meanwhile,
>>> please take a look at the attached if you have the time. I rewrote much
>>> of the docs changes, and some comments.
>
> Looks fine to me.

Ok, committed with the changes to the error handling. I refactored the
logic to construct a human-readable string from pclose_check() to a new
function, and used that.

Thanks for the patch, and thanks Amit for the review!

- Heikki