Re: Function array_agg(array) - Mailing list pgsql-hackers
From | Pavel Stehule |
---|---|
Subject | Re: Function array_agg(array) |
Date | |
Msg-id | CAFj8pRA8fLBo2jmm6tPOrHVwvTcsfWQarE8Pn7R0rc3gzcVTBg@mail.gmail.com Whole thread Raw |
In response to | Re: Function array_agg(array) (Pavel Stehule <pavel.stehule@gmail.com>) |
Responses |
Re: Function array_agg(array)
|
List | pgsql-hackers |
2014-10-25 8:33 GMT+02:00 Pavel Stehule <pavel.stehule@gmail.com>:
2014-10-25 8:19 GMT+02:00 Ali Akbar <the.apaan@gmail.com>: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 nowI 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_transfnprobably 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"
I am thinking so change of makeArrayResult is wrong. It is developed for 1D array. When we expect somewhere ND result now, then we should to use makeMdArrayResult there. So makeArrayResult should to return 1D array in all cases. Else a difference between makeArrayResult and makeMdArrayResult hasn't big change and we have a problem with naming.
Regards
Pavel
Pavel
--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: