On Sun, Dec 15, 2013 at 9:29 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
David Rowley <dgrowleyml@gmail.com> writes: > I guess the answer for the people that complain about slowness could be > that they create their own aggregate function which implements float4pl as > the trans function and float4mi as the negative trans function. They can > call it SUMFASTBUTWRONG() if they like. Perhaps it would be worth a note in > the documents for this patch?
I think it would be an absolutely perfect documentation example to show how to set up such an aggregate (and then point out the risks, of course).
I've attached an updated patch which includes some documentation.
I've also added support for negfunc in CREATE AGGREGATE. Hopefully that's an ok name for the option, but if anyone has any better ideas please let them be known.
For the checks before the aggregate is created, I put these together quite quickly and I think I'm still missing a check to ensure that the strict property for the trans and negtrans functions are set to the same thing, though I do have a runtime check for this, it's likely bad to wait until then to tell the user about it.
> As for numeric, I did start working on this just after I posted the > original patch and before I saw your comment about it. I did end up making > do_numeric_desperse() which was to be the reverse of do_numeric_accum(), > but I got stuck on the equivalent of when do_numeric_accum() > does mul_var(&X, &X, &X2, X.dscale * 2);
Ummm ... why doesn't it work to just use numeric_add and numeric_sub, exactly parallel to the float case?
I've not quite got back to this yet and I actually pulled out my initial try at this thinking that we didn't want it because it was affecting the scale. The transition function for SUM numeric does not seem to use numeric_add, it uses numeric_avg_accum as the transition function which lets do_numeric_accum do the hard work and that just does add_var. I changes these add_var's to sub_var's and altered the initial value to flip the sign so that NULL,10 would be -10 instead of 10. I think that's all it needs, and I guess I leave the dscale as is in this situation then?