Hi,

I started reviewing this patch. Gone through the mail thread discussion to
understand the need of the patch. With patch there is significant
performance
improvement in case of update for the array scalar variable.

- Patch gets apply cleanly on master branch
- make, make install and make check works fine

Did code walk through, code change looks good to me. I was doing some
testing
in the area of scalar variable array and domain type. For the big array size
noticed significant performance improvement. So far haven't found any issues
with the code.

Patch looks good to me.

Just FYI.. Do need to remove array_fast_update GUC in the final version of
commit.

Regards,
Rushabh


On Mon, Oct 7, 2013 at 7:52 PM, Pavel Stehule <pavel.steh...@gmail.com>wrote:

>
>
>
> 2013/10/7 Andres Freund <and...@2ndquadrant.com>
>
>> On 2013-10-07 16:00:54 +0200, Pavel Stehule wrote:
>> >                               /*
>> >                                * We need to do subscript evaluation,
>> which might require
>> > @@ -4321,6 +4322,14 @@ exec_assign_value(PLpgSQL_execstate *estate,
>> >                                       oldarrayval = (ArrayType *)
>> DatumGetPointer(oldarraydatum);
>> >
>> >                               /*
>> > +                              * support fast update for array scalar
>> variable is enabled only
>> > +                              * when target is a scalar variable and
>> variable holds a local
>> > +                              * copy of some array.
>> > +                              */
>> > +                             inplace_update = (((PLpgSQL_datum *)
>> target)->dtype == PLPGSQL_DTYPE_VAR
>> > +                                                         &&
>> ((PLpgSQL_var *) target)->freeval);
>> > +
>> > +                             /*
>> >                                * Build the modified array value.
>> >                                */
>>
>> Will this recognize if the local Datum is just a reference to an
>> external toast Datum (i.e. varattrib_1b_e)?
>>
>>
> this works on plpgsql local copies only (when cleaning is controlled by
> plpgsql) - so it should be untoasted every time.
>
> btw when I tested this patch I found a second plpgsql array issue -
> repeated untoasting :( and we should not forget it.
>
>
>
>
>> I don't know much about plpgsql's implementation, so please excuse if
>> the question is stupid.
>>
>> Greetings,
>>
>> Andres Freund
>>
>> --
>>  Andres Freund                     http://www.2ndQuadrant.com/
>>  PostgreSQL Development, 24x7 Support, Training & Services
>>
>
>


-- 
Rushabh Lathia

Reply via email to