2014-12-16 6:27 GMT+07:00 Tomas Vondra <tv@fuzzy.cz>:
On 15.12.2014 22:35, Jeff Janes wrote: > On Sat, Nov 29, 2014 at 8:57 AM, Tomas Vondra <tv@fuzzy.cz
> <mailto:tv@fuzzy.cz>> wrote: > > Hi, > > Attached is v2 of the patch lowering array_agg memory requirements. > Hopefully it addresses the issues issues mentioned by TL in this thread > (not handling some of the callers appropriately etc.). > > > Hi Tomas, > > When configured --with-libxml I get compilation errors: > > xml.c: In function 'xml_xpathobjtoxmlarray': > xml.c:3684: error: too few arguments to function 'accumArrayResult' > xml.c:3721: error: too few arguments to function 'accumArrayResult' > xml.c: In function 'xpath': > xml.c:3933: error: too few arguments to function 'initArrayResult' > xml.c:3936: error: too few arguments to function 'makeArrayResult' > > And when configured --with-perl, I get: > > plperl.c: In function 'array_to_datum_internal': > plperl.c:1196: error: too few arguments to function 'accumArrayResult' > plperl.c: In function 'plperl_array_to_datum': > plperl.c:1223: error: too few arguments to function 'initArrayResult' > > Cheers,
Thanks, attached is a version that fixes this.
Just fast-viewing the patch.
The patch is not implementing the checking for not creating new context in initArrayResultArr. I think we should implement it also there for consistency (and preventing future problems).
Looking at the modification in accumArrayResult* functions, i don't really comfortable with:
Code that calls accumArrayResult* after explicitly calling initArrayResult* must always passing subcontext, but it has no effect.
All existing codes that calls accumArrayResult must be changed.
Just an idea: why don't we minimize the change in API like this:
Adding parameter bool subcontext, only in initArrayResult* functions but not in accumArrayResult*
Code that want to not creating subcontext must calls initArrayResult* explicitly.
Other codes that calls directly to accumArrayResult can only be changed in the call to makeArrayResult* (with release=true parameter). In places that we don't want to create subcontext (as in array_agg_transfn), modify it to use initArrayResult* before calling accumArrayResult*.
