Re: Patch: propose to include 3 new functions into intarray and intagg

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