Hi,
Gregory Stark wrote:
> Regarding the patch listed on the commitfest "3 new functions into intarray
> and intagg" (which I just noticed has a reviewer listed -- doh):
..well, just add your name as well, no?
> I definitely like the int_array_append_aggregate function but I don't see
> anything int[] specific about it. We should be able to have a generic
> array_union() aggregate which uses the same IsA(fcinfo->context, AggState)
> trick to scribble on its state variable. It don't even see any reason it
> couldn't work for arrays of varlenas, though it would take a bit of
> restructuring.
Yeah, the same idea was bugging me. Doesn't such code already exist?
> So I would be definitely for a adding this to core if it were rewritten to
> work with generic arrays which, unless there are problems I'm not seeing, I
> don't think would be very hard.
>
> As far as detailed code commentary the only thing which jumps out at me is
> that it's using MemoryContextAlloc to grow the array instead of repalloc which
> seems like a waste. This isn't a new thing though, it was how intagg was
> written and this patch just didn't change it.
Oh, good catch.
> I'm not against putting more functions into intagg and intarray and bidx and
> the grouping/counting thing seem like they might be useful functionality. but
> I have a feeling others might feel differently.
The naming 'bidx' seems a bit weired to me, but otherwise I'm also
optimistic about it.
Regards
Markus Wanner
