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-12-11 05:41:08
Message-ID: BF2827DCCE55594C8D7A8F7FFD3AB7713DDB0B26@SZXEML508-MBX.china.huawei.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 9th December, Amit Khandelkar wrote:

>1. slashcopyissuev1.patch :- This patch fixes the \COPY issue.
>You have removed the if condition in this statement, mentioning that it is always true now:
>- if (copystream == pset.cur_cmd_source)
>- pset.lineno++;
>+ pset.lineno++;
>
>But copystream can be different than pset.cur_cmd_source , right ?

As per the earlier code, condition result was always true. So pset.lineno was always incremented.
In the earlier code pset.cur_cmd_source was sent as parameter to function and inside the function same parameter was used with the name copystream. So on entry of this function both will be one and same.
I checked inside the function handleCopyIn, both of these parameters are not changing before above check. Also since pset is specific to single session, so it cannot change concurrently.
Please let me know, if I am missing something.

>+ FILE *copyStream; /* Stream to read/write for copy command */
>
>There is no tab between FILE and *copystream, hence it is not aligned.

OK. I shall change accodingly.

>2. initialcopyissuev1_ontopofslashcopy.patch : Fix for "COPY table FROM STDIN/STDOUT doesn't show count tag".
>The following header comments of ProcessResult() need to be modified:
>* Changes its argument to point to the last PGresult of the command string,
>* or NULL if that result was for a COPY FROM STDIN or COPY TO STDOUT.

OK. I shall change accodingly.

>Regression results show all passed.
>Other than this, the patch needs a new regression test.

I had checked the existing regression test cases and observed that it has already got all kind of test cases. Like
copy....stdin,
copy....stdout,
\copy.....stdin
\copy.....stdout.

But since as regression framework runs in "quite i.e. -q" mode, so it does not show any message except query output.
So our new code change does not impact regression framework.

Please let me know if you were expecting any other test cases?

>I don't think we need to do any doc changes, because the doc already mentions that COPY should show the COUNT tag, and does not mention anything specific to client-side COPY.
OK.

Please provide you opinion, based on which I shall prepare new patch and share the same.

Thanks and Regards,
Kumar Rajeev Rastogi

Browse pgsql-hackers by date

  From Date Subject
Next Message KONDO Mitsumasa 2013-12-11 06:14:08 Re: Optimize kernel readahead using buffer access strategy
Previous Message Tom Lane 2013-12-11 05:10:38 Re: plpgsql_check_function - rebase for 9.3