Re: ToDo: allow to get a number of processed rows by COPY statement [Review of Patch]

On 27.09.2012 23:58, Pavel Stehule wrote:
> Hello
>
> I reduced this patch as just plpgsql (SPI) problem solution.
>
> Correct generic solution needs a discussion about enhancing our libpq
> interface - we can take a number of returned rows, but we should to
> get number of processed rows too. A users should to parse this number
> from completationTag, but this problem is not too hot and usually is
> not blocker, because anybody can process completationTag usually.
>
> So this issue is primary for PL/pgSQL, where this information is not
> possible. There is precedent - CREATE AS SELECT (thanks for sample
> :)), and COPY should to have same impact on ROW_COUNT like this
> statement (last patch implements it).

The comment where CREATE AS SELECT does this says:

>     /*
>      * CREATE TABLE AS is a messy special case for historical
>      * reasons.  We must set _SPI_current->processed even though
>      * the tuples weren't returned to the caller, and we must
>      * return a special result code if the statement was spelled
>      * SELECT INTO.
>      */

That doesn't sound like a very good precedent that we should copy to 
COPY. I don't have a problem with this approach in general, though. But 
if we're going to make it normal behavior, rather than an ugly 
historical kluge, that utility statements can return a row count without 
returning the tuples, we should document that. The above comment ought 
to be changed, and the manual section about SPI_execute needs to mention 
the possibility that SPI_processed != 0, but SPI_tuptable == NULL.

Regarding the patch itself:

> +                else if (IsA(stmt, CopyStmt))
> +                {
> +                    /*
> +                     * usually utility statements doesn't return a number
> +                     * of processed rows, but COPY does it.
> +                     */
> +                    Assert(strncmp(completionTag, "COPY  ", 5) == 0);
> +                    _SPI_current->processed = strtoul(completionTag + 5,
> +                                                      NULL, 10);
> +                }
>                  else
>                      res = SPI_OK_UTILITY;

'res' is not set for a CopyStmt, and there's an extra space in "COPY  " 
in the Assert.

- Heikki