Re: COPY table FROM STDIN doesn't show count tag

From: Rajeev rastogi <rajeev(dot)rastogi(at)huawei(dot)com>
To: Amit Khandekar <amit(dot)khandekar(at)enterprisedb(dot)com>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: COPY table FROM STDIN doesn't show count tag
Date: 2013-11-20 12:10:38
Message-ID: BF2827DCCE55594C8D7A8F7FFD3AB7713DDAC5A5@SZXEML508-MBX.china.huawei.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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(at)linux-ltr9:~/9.4gitcode/install/bin<mailto:rajeev(at)linux-ltr9:~/9.4gitcode/install/bin>> ./psql -d postgres -c "\copy tbl to 'new.txt';insert into tbl values(55);"
>>rajeev(at)linux-ltr9:~/9.4gitcode/install/bin<mailto:rajeev(at)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

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Dilip kumar 2013-11-20 12:19:59 Re: Logging WAL when updating hintbit
Previous Message Andres Freund 2013-11-20 12:00:35 Re: Data corruption issues using streaming replication on 9.0.14/9.2.5/9.3.1