Re: Function array_agg(array) - Mailing list pgsql-hackers
From | Pavel Stehule |
---|---|
Subject | Re: Function array_agg(array) |
Date | |
Msg-id | CAFj8pRCa7bXFjnj6iADuf30oyajLzsouM6nEs=hnM_XHy6et-g@mail.gmail.com Whole thread Raw |
In response to | Re: Function array_agg(array) (Ali Akbar <the.apaan@gmail.com>) |
Responses |
Re: Function array_agg(array)
|
List | pgsql-hackers |
2014-10-25 8:19 GMT+02:00 Ali Akbar <the.apaan@gmail.com>:
makeArrayResult1 - I have no better name now
2014-10-25 10:29 GMT+07:00 Ali Akbar <the.apaan@gmail.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.stehule@gmail.com>:Hi AliI 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"
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
pgsql-hackers by date: