Thread: Re: [PATCHES] patch implementing the multi-argument aggregates (SOC project)
"Sergey E. Koposov" <math@sai.msu.ru> writes:
> Since the feature freeze is in a few days, I'm sending the first iteration
> of my patch implementing the multi-argument aggregates (PolyArgAgg) (SOC
> project)
This patch is nowhere near ready for submission :-(. Most of the
comments seem to be "I don't know what to do here" ...
A general hint on the polymorphic stuff is that you should be able to
exactly duplicate what's done for polymorphic functions --- or even
better, get rid of the separate code for aggregates and just invoke
the existing logic for functions. (You might need to refactor code
a little bit to separate out the common functionality.)
Instead of copying data inside advance_transition_function, it might
be better for the caller to store the values into the right fields
of a temporary FunctionCallInfoData struct, and just pass that to
advance_transition_function.
The names for the new aggregates seem a bit, how to say, terse and
unfriendly. SQL generally tends to a more verbose style of naming.
regards, tom lane
On Mon, 24 Jul 2006, Tom Lane wrote:
> "Sergey E. Koposov" <math@sai.msu.ru> writes:
>> Since the feature freeze is in a few days, I'm sending the first iteration
>> of my patch implementing the multi-argument aggregates (PolyArgAgg) (SOC
>> project)
>
> This patch is nowhere near ready for submission :-(. Most of the
:-(
But now at least I know that...
> comments seem to be "I don't know what to do here" ...
>
No that's not quite true... I have only ~ 2-3 such comments, all others
just express that I marked the places where I've had any little doubts
and which I'll check additionally...
> A general hint on the polymorphic stuff is that you should be able to
> exactly duplicate what's done for polymorphic functions --- or even
> better, get rid of the separate code for aggregates and just invoke
> the existing logic for functions. (You might need to refactor code
> a little bit to separate out the common functionality.)
>
> Instead of copying data inside advance_transition_function, it might
> be better for the caller to store the values into the right fields
> of a temporary FunctionCallInfoData struct, and just pass that to
> advance_transition_function.
Thank you for the hints, I'll think about them...
> The names for the new aggregates seem a bit, how to say, terse and
> unfriendly. SQL generally tends to a more verbose style of naming.
>
The names for the functions came from SQL 2003 standart...
Regards,
Sergey
*******************************************************************
Sergey E. Koposov
Max Planck Institute for Astronomy/Sternberg Astronomical Institute
Tel: +49-6221-528-349
Web: http://lnfm1.sai.msu.ru/~math
E-mail: math@sai.msu.ru
Re: [PATCHES] patch implementing the multi-argument aggregates (SOC project)
From
Gregory Stark
Date:
Tom Lane <tgl@sss.pgh.pa.us> writes: > This patch is nowhere near ready for submission :-(. Most of the > comments seem to be "I don't know what to do here" ... Just to be clear, I think what Tom's saying it's not ready to be *applied*. Sending patches to this list early and often during development is generally encouraged. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com
Gregory Stark <gsstark@mit.edu> writes:
> Tom Lane <tgl@sss.pgh.pa.us> writes:
>> This patch is nowhere near ready for submission :-(. Most of the
>> comments seem to be "I don't know what to do here" ...
> Just to be clear, I think what Tom's saying it's not ready to be *applied*.
> Sending patches to this list early and often during development is generally
> encouraged.
Indeed, but if that was the point we should have been seeing drafts of
this patch long ago. I interpreted Sergey's submission as "getting this
in before feature freeze", and in that context the bar is higher. If a
patch isn't pretty nearly ready to be applied when the freeze deadline
arrives, we won't wait around for it.
regards, tom lane