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
