2014-12-16 10:47 GMT+07:00 Ali Akbar <the.ap...@gmail.com>: > > > 2014-12-16 6:27 GMT+07:00 Tomas Vondra <t...@fuzzy.cz>: >> >> On 15.12.2014 22:35, Jeff Janes wrote: >> > On Sat, Nov 29, 2014 at 8:57 AM, Tomas Vondra <t...@fuzzy.cz >> > <mailto:t...@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: 1. Code that calls accumArrayResult* after explicitly calling initArrayResult* must always passing subcontext, but it has no effect. 2. All existing codes that calls accumArrayResult must be changed. Just an idea: why don't we minimize the change in API like this: 1. Adding parameter bool subcontext, only in initArrayResult* functions but not in accumArrayResult* 2. 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*. What do you think? Regards, -- Ali Akbar