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

From: Gregory Stark <stark(at)enterprisedb(dot)com>
To: "Dmitry Koterov" <dmitry(at)koterov(dot)ru>
Cc: Postgres <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Patch: propose to include 3 new functions into intarray and intagg
Date: 2008-09-05 11:16:08
Message-ID: 87hc8u4zd2.fsf@oxford.xeocode.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


Regarding the patch listed on the commitfest "3 new functions into intarray
and intagg" (which I just noticed has a reviewer listed -- doh):

http://archives.postgresql.org/message-id/d7df81620808130429l2a75c895g5dd6fe8ae64cc23e@mail.gmail.com

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.

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.

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.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's RemoteDBA services!

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Simon Riggs 2008-09-05 11:18:11 Re: Need more reviewers!
Previous Message Markus Wanner 2008-09-05 11:13:40 Re: WIP: Column-level Privileges