2012/10/1 Heikki Linnakangas <hlinnakan...@vmware.com>:
> 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.
>

fixed patch

Regards

Pavel Stehule


> - Heikki

Attachment: copy-processed-rows-simple2.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to