On 12th December 2013, Rajeev Rastogi Wrote:
>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.

I ran pgindent on settings.h file but found no issue reported. Seems tab is not 
mandatory in between variable declaration.
Even for other parameters also in psqlSetting structure space instead of tab is 
being used.
So seems no change required for this. Please confirm.


>>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.
I have changed it in the latest patch.

>>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?

Summary of two patches:

1.      slashcopyissuev1.patch :- No change in this patch (same as earlier).

2.      Initialcopyissuev2_ontopofslashcopy.patch : This patch is modified to 
change comment as per above review comments.

Please provide your opinion or let me know if any other changes are required.

Thanks and Regards,
Kumar Rajeev Rastogi


Attachment: slashcopyissuev1.patch
Description: slashcopyissuev1.patch

Attachment: initialcopyissuev2_ontopofslashcopy.patch
Description: initialcopyissuev2_ontopofslashcopy.patch

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to