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