From: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> |
---|---|
To: | Ali Akbar <the(dot)apaan(at)gmail(dot)com> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Function array_agg(array) |
Date: | 2014-10-25 06:33:12 |
Message-ID: | CAFj8pRCa7bXFjnj6iADuf30oyajLzsouM6nEs=hnM_XHy6et-g@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
2014-10-25 8:19 GMT+02:00 Ali Akbar <the(dot)apaan(at)gmail(dot)com>:
> 2014-10-25 10:29 GMT+07:00 Ali Akbar <the(dot)apaan(at)gmail(dot)com>:
>
>> I fixed small issue in regress tests and I enhanced tests for varlena
>>> types and null values.
>>
>> Thanks.
>>
>> it is about 15% faster than original implementation.
>>
>> 15% faster than array_agg(scalar)? I haven't verify the performance, but
>> because the internal array data and null bitmap is copied as-is, that will
>> be faster.
>>
>> 2014-10-25 1:51 GMT+07:00 Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>:
>>
>>> Hi Ali
>>>
>>
>>> I checked a code. I am thinking so code organization is not good.
>>> accumArrayResult is too long now. makeMdArrayResult will not work, when
>>> arrays was joined (it is not consistent now). I don't like a usage of
>>> state->is_array_accum in array_userfunc.c -- it is signal of wrong
>>> wrapping.
>>>
>> Yes, i was thinking the same. Attached WIP patch to reorganizate the
>> code. makeMdArrayResult works now, with supplied arguments act as override
>> from default values calculated from ArrayBuildStateArray.
>>
>> In array_userfunc.c, because we don't want to override ndims, dims and
>> lbs, i copied array_agg_finalfn and only change the call to
>> makeMdArrayResult (we don't uses makeArrayResult because we want to set
>> release to false). Another alternative is to create new
>> makeArrayResult-like function that has parameter bool release.
>>
>
> adding makeArrayResult1 (do you have better name for this?), that accepts
> bool release parameter. array_agg_finalfn becomes more clean, and no
> duplicate code in array_agg_anyarray_finalfn.
>
makeArrayResult1 - I have no better name now
I found one next minor detail.
you reuse a array_agg_transfn function. Inside is a message
"array_agg_transfn called in non-aggregate context". It is not correct for
array_agg_anyarray_transfn
probably specification dim and lbs in array_agg_finalfn is useless, when
you expect a natural result. A automatic similar to
array_agg_anyarray_finalfn should work there too.
makeMdArrayResultArray and appendArrayDatum should be marked as "static"
>
>> next question: there is function array_append(anyarray, anyelement).
>>> Isn't time to define array_append(anyarray, anyarray) now?
>>>
>>
>> There is array_cat(anyarray, anyarray):
>>
>> /*-----------------------------------------------------------------------------
>> * array_cat :
>> * concatenate two nD arrays to form an nD array, or
>> * push an (n-1)D array onto the end of an nD array
>>
>> *----------------------------------------------------------------------------
>> */
>>
>
> Regards,
> --
> Ali Akbar
>
From | Date | Subject | |
---|---|---|---|
Next Message | Pavel Stehule | 2014-10-25 06:54:24 | Re: Function array_agg(array) |
Previous Message | Ali Akbar | 2014-10-25 06:19:34 | Re: Function array_agg(array) |