Skip site navigation (1) Skip section navigation (2)

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

From: Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: Amit Kapila <amit(dot)kapila(at)huawei(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Bruce Momjian <bruce(at)momjian(dot)us>
Subject: Re: ToDo: allow to get a number of processed rows by COPY statement [Review of Patch]
Date: 2012-10-01 07:29:31
Message-ID: 5069465B.5010302@vmware.com (view raw or flat)
Thread:
Lists: pgsql-hackers
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


In response to

Responses

pgsql-hackers by date

Next:From: Pavel StehuleDate: 2012-10-01 07:33:04
Subject: Re: ToDo: allow to get a number of processed rows by COPY statement [Review of Patch]
Previous:From: Nozomi AnzaiDate: 2012-10-01 07:28:16
Subject: Re: 64-bit API for large object

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group