Pavel Stehule <pavel.stehule@gmail.com> writes:
> 2014-10-27 11:20 GMT+01:00 Ali Akbar <the.apaan@gmail.com>:
>> [ array_agg_anyarray-13.patch ]
> This patch is ready for commit
I've committed this after some significant modifications.
I did not like the API chosen, specifically the business about callers
needing to deal with both accumArrayResult and accumMdArray, because that
seemed pretty messy and error-prone. There was in fact at least one
calling-logic error, namely that the empty array created by nodeSubplan.c
when there were zero inputs would have the wrong element type for the
array-input case.
It seemed to me that the best fix for that would be to push the empty
array creation special case into the accumArrayResult function family.
After some thought about how to do that, I settled on adding an "init"
function that can create an ArrayBuildState with no data yet put into
it; the main purpose is just to save the info about the input datatype.
Then, if makeArrayResult receives the ArrayBuildState with still no
data in it, it can create the appropriate empty array. (This turned
out to be a better idea than I first realized, as both xml.c and plperl.c
had coding patterns that could be made significantly less ugly with a
convention that the ArrayBuildState is always there.)
After doing that, I adjusted your new functions to have similar APIs,
and then added a switching layer on top for use by callers that want
to support both the scalar and array cases. This made the adjustments
for array subplans more or less one-liner changes in the relevant
places. We could possibly have unified the array_agg support functions
as well, but I didn't see much point in that given that we need two
sets of pg_proc entries to make type resolution work properly.
There were a number of other smaller issues too, like not being cautious
about memory allocation (the submitted code could both fail to allocate
enough, and compute an allocation request that would overflow an int).
regards, tom lane
