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

On 25 November 2013, Amit Khandekar <amit(dot)khandekar(at)enterprisedb(dot)com<mailto:amit(dot)khandekar(at)enterprisedb(dot)com>> wrote:
>>>Ok. we will then first fix the \COPY TO issue where it does not revert back the overriden psql output file handle. Once this is solved, fix for both COPY FROM and COPY TO, like how it is done in the patch earlier (copydefectV2.patch).

>>I analyzed the solution to fix \COPY TO issue but unfortunately I observed that do_copy is already resetting the value of cur_cmd_source and queryFout but before that itself result status is printed. So we'll have to reset the value before result status is
>>being displayed.
>>So as other alternative solutions, I have two approaches:

>>1. We can store current file destination queryFout in some local variable and pass the same to SendQuery function as a parameter. Same can be used to reset the value of queryFout after return from ProcessResult

>>From all other callers of SendQuery , we can pass NULL value for this new parameter.
>>2. We can add new structure member variable FILE *prevQueryFout in structure "struct _psqlSettings", which hold the value of queryFout before being changed in do_copy. And then same can be used to reset value in SendQuery or ProcessResult.

>I think approach #2 is fine. Rather than prevQueryFout, I suggest defining a separate FILE * handle for COPY. I don't see any other client-side command that uses its own file pointer for reading and writing, like how COPY does. And this handle has
> nothing to do with pset stdin and stdout. So we can have this special _psqlSettings->copystream specifically for COPY. Both handleCopyIn() and handleCopyOut() will be passed pset.copystream. In do_copy(), instead of overriding
>pset.queryFout, we can set pset.copystream to copystream, or to stdin/stdout if copystream is NULL.

OK. I have revised the patch as per the discussion. Now if \copy command is called then, we are setting the appropriate value of _psqlSettings->copystream in do_copy and same is being used inside handleCopyIn() and handleCopyOut(). Once the \copy command execution finishes, we are resetting the value of _psqlSettings->copystream to NULL. And if COPY(No slash) command is used, then in that case _psqlSettings->copystream will be NULL. So based on this value being NULL, copyStream will be assigned as stdout/stdin depending on TO/FROM respectively inside the function handleCopyOut()/handleCopyIn().

Also in order to address the queries like
./psql -d postgres -c "\copy tbl to '/home/rajeev/9.4gitcode/install/bin/data/temp.txt'; copy tbl from stdin;"
Inside the function ProcessResult, we check that if it is the second cycle and result status is COPY OUT or IN, then we reset the value of _psqlSettings->copystream to NULL, so that it can take the value as stdout/stdin for further processing.

Please provide your opinion.

Thanks and Regards,
Kumar Rajeev Rastogi

On 25 November 2013 15:25, Rajeev rastogi <rajeev(dot)rastogi(at)huawei(dot)com> wrote:

> OK. I have revised the patch as per the discussion.
>
Could you please submit only the \COPY fix first ? The attached patch also
contains the fix for the original COPY status fix.

Now if \copy command is called then, we are setting the appropriate value
> of _psqlSettings->copystream in do_copy and same is being used inside
> handleCopyIn() and handleCopyOut(). Once the \copy command execution
> finishes, we are resetting the value of _psqlSettings->copystream to NULL.
> And if COPY(No slash) command is used, then in that case
> _psqlSettings->copystream will be NULL. So based on this value being NULL,
> copyStream will be assigned as stdout/stdin depending on TO/FROM
> respectively inside the function handleCopyOut()/handleCopyIn().
>

> Also in order to address the queries like
>
*./psql -d postgres -c "\copy tbl to
> '/home/rajeev/9.4gitcode/install/bin/data/temp.txt'; copy tbl from stdin;"*
>
> Inside the function ProcessResult, we check that if it is the second cycle
> and result status is COPY OUT or IN, then we reset the value of
> _psqlSettings->copystream to NULL, so that it can take the value as
> stdout/stdin for further processing.
>
>
>

Yes, that's right, the second cycle should not use pset.copyStream.

handleCopyOut(PGconn *conn, FILE *copystream, PGresult **res)
{
bool OK = true;
char *buf;
int ret;
- PGresult *res;
+
+ if (!copystream)
+ copystream = stdout;

It should use pset.queryFout if it's NULL. Same in hadleCopyIn().
Otherwise, the result of the following command goes to stdout, when it
should go to the output file :
psql -d postgres -o /tmp/p.out -c "copy tab to stdout"

+ /*
+ * If this is second copy; then it will be
definately not \copy,
+ * and also it can not be from any user
given file.
+ * So reset the value of copystream to
NULL, so that read/wrie
+ * happens from stdin/stdout.
+ */
+ if (!first_cycle)
+ pset.copyStream = NULL;

Let ProcessResult() not change pset.copyStream. Let only do_copy() update
it. Instead of the above location, I suggest, just before calling
handleCopyOut/In(), we decide what to pass them as their copyStream
parameter depending upon whether it is first cycle or not.

> Please provide your opinion.
>
>
>
> Thanks and Regards,
>
> Kumar Rajeev Rastogi
>
>
>