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: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Heikki Linnakangas <hlinnakangas(at)vmware(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:33:04
Message-ID: CAFj8pRB5vqG+RcKByct_8EiYYfxF5_u09K5THRz+iNztJohi-g@mail.gmail.com (view raw or flat)
Thread:
Lists: pgsql-hackers
2012/10/1 Heikki Linnakangas <hlinnakangas(at)vmware(dot)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.

res is issue,

Extra space is correct, we would to break, when completetionTag is changed.

Pavel

>
> - Heikki


In response to

pgsql-hackers by date

Next:From: Dimitri FontaineDate: 2012-10-01 07:44:28
Subject: Re: [9.1] 2 bugs with extensions
Previous:From: Heikki LinnakangasDate: 2012-10-01 07:29:31
Subject: Re: ToDo: allow to get a number of processed rows by COPY statement [Review of Patch]

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