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

Reply via email to