On 20 November, Amit Khandekar wrote:
>>>>I hope you meant to write test case as psql -d postgres -c "\copy tab from 
>>>>stdin; insert into tab values ('lll', 3)", as if we are reading from file, 
>>>>then the above issue does not come.
>>>I meant COPY with a slash. \COPY is equivalent to COPY FROM STDIN. So the 
>>>issue can also be reproduced by :
>>>\COPY tab from 'client_filename' ...

 >>>>I have modified the patch as per your comment and same is attached with 
 >>>>this mail.

>>>Thanks. The COPY FROM looks good.
>>OK..Thanks
>>>With the patch applied, \COPY TO 'data_file' command outputs the  COPY 
>>>status into the data file, instead of printing it in the psql session.
>>>postgres=# \copy tab to '/tmp/fout';
>>>postgres=#
 >>>$ cat /tmp/fout
>>>ee      909
>>>COPY 1
>>>This is probably because client-side COPY overrides the pset.queryFout with 
>>>its own destination file, and while printing the COPY status, the overridden 
>>>file pointer is not yet reverted back.
 >>This looks to be an issue without our new patch also. Like I tried following 
 >>command and output was as follows:
>>rajeev@linux-ltr9:~/9.4gitcode/install/bin<mailto:rajeev@linux-ltr9:~/9.4gitcode/install/bin>>
>> ./psql -d postgres -c "\copy tbl to 'new.txt';insert into tbl values(55);"
>>rajeev@linux-ltr9:~/9.4gitcode/install/bin<mailto:rajeev@linux-ltr9:~/9.4gitcode/install/bin>>
>> cat new.txt
>>5
>>67
>>5
>>67
>>2
>>2
>>99
>>1
>>1
>INSERT 0 1

>Ok. Yes it is an existing issue. Because we are now printing the COPY status 
>even for COPY TO, the existing issue surfaces too easily with the patch. \COPY 
>TO is a pretty common scenario. And it does not have to have a subsequent 
>another command
>to reproduce the issue Just a single \COPY TO command reproduces the issue.
>>I have fixed the same as per your suggestion by resetting the pset.queryFout 
>>after the function call "handleCopyOut".
>>!                                          pset.queryFout = stdout;

>The original pset.queryFout may not be stdout. psql -o option can override the 
>stdout default.

>I think solving the \COPY TO part is going to be a  different (and an 
>involved) issue to solve than the COPY FROM. Even if we manage to revert back 
>the queryFout, I think ProcessResult() is not the right place to do it. 
>ProcessResult() should not
> assume that  somebody else has changed queryFout. Whoever has changed it 
> should revert it. Currently, do_copy() is indeed doing this correctly:

>          save_file = *override_file;
>          *override_file = copystream;
>          success = SendQuery(query.data);
>          *override_file = save_file;

>But the way SendQuery() itself processes the results and prints them, is 
>conflicting with the above.

>So I think it is best to solve this as a different issue, and we should , for 
>this commitfest,  fix only COPY FROM. Once the \COPY existing issue is solved, 
>only then we can start printing the \COPY TO status as well.

You mean to say that I should change the patch to keep only COPY FROM related 
changes and remove changes related to COPY TO.
If yes, then I shall change the patch accordingly  and also mention same in 
documentation also.
Please let me know about this so that I can share the modified patch.

Thanks and Regards,
Kumar Rajeev Rastogi

Reply via email to