Re: ToDo: fast update of arrays with fixed length fields for PL/pgSQL

From: Rushabh Lathia <rushabh(dot)lathia(at)gmail(dot)com>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: Andres Freund <andres(at)2ndquadrant(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: ToDo: fast update of arrays with fixed length fields for PL/pgSQL
Date: 2013-11-21 09:21:40
Message-ID: CAGPqQf3RB6s+AdKuF6OyzkegVSKQbm=N_F-XYQfe02RGJHXOfw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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(dot)stehule(at)gmail(dot)com>wrote:

>
>
>
> 2013/10/7 Andres Freund <andres(at)2ndquadrant(dot)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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Atri Sharma 2013-11-21 10:04:52 Re: WITHIN GROUP patch
Previous Message Dean Rasheed 2013-11-21 09:15:26 Re: WITH ORDINALITY versus column definition lists