Hi Amit, Thank you for the review.
> From: Amit Kapila [mailto:amit.kap...@huawei.com] > >> Test case issues: > >> ------------------ > >> 1. "Broken pipe" is not handled in case of psql "\copy" command; > >> Issue are as follows: > >> Following are verified on SuSE-Linux 10.2. > >> 1) psql is exiting when "\COPY xxx TO" command is issued and > command/script is not found > >> When popen is called in write mode it is creating valid > file descriptor and when it tries to write to file "Broken pipe" error is > > coming which is not handled. > >> psql# \copy pgbench_accounts TO PROGRAM > '../compress.sh pgbench_accounts4.txt' > >> 2) When "\copy" command is in progress then program/command is > killed/"crashed due to any problem" > >> psql is exiting. > > >This is a headache. I have no idea how to solve this. > > I think we can keep it for committer to take a call on this issue. Agreed. > I have found few more minor issues as below: > > 1. The comment above do_copy can be modified to address the new > functionality it can handle. > /* > * Execute a \copy command (frontend copy). We have to open a file, then > * submit a COPY query to the backend and either feed it data from the > * file or route its response into the file. > */ > bool > do_copy(const char *args) Done. > 2. > @@ -256,8 +273,14 @@ do_copy(const char *args) > + if (options->file == NULL && options->program) > + { > + psql_error("program is not supported to stdout/pstdout or > from stdin/pstdin\n"); > + return false; > + } > > should call free_copy_options(options); before return false; Good catch! Done. > 3. \copy command doesn't need semicolon at end, however it was working > previous to your patch, but > now it is giving error. > postgres=# \copy t1 from 'e:\pg_git_code\Data\t1_Data.txt'; > e:/pg_git_code/Data/t1_Data.txt';: No such file or directory > e:/pg_git_code/Data/t1_Data.txt';: No such file or directory Sorry, I've fixed the bug. > 4. Please check if OpenPipeStream() it needs to call > if (ReleaseLruFile()), OpenPipeStream() calls ReleaseLruFile() by itself if necessary. > 5. Following in copy.sgml can be changed to make more meaningful as the > first line looks little adhoc. > + <para> > + The command that input comes from or that output goes to. > + The command for COPY FROM, which input comes from, must write its > output > + to standard output. The command for COPY TO, which output goes to, > must > + read its input from standard input. > + </para> I've struggled to make the document more meaningful. > 6. Can we have one example of this new syntax, it can make it more > meaningful. Done. Sorry for the long delay. Best regards, Etsuro Fujita > With Regards, > Amit Kapila. > >
copy-popen-20130220.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
