Florian Pflug <fgp@phlo.org> writes:
> My argument is that is costs us more complexity to duplicate everything
> for the invertible case, *and* the result seems less flexible - not
> from the POV of aggregate implementations, but from the POV of future
> extensions.
[ shrug... ] You can argue against any feature whatsoever by claiming
that it might possibly conflict with something we would wish to do
sometime in the future. I think you need to have a much more concrete
argument about specific issues this will cause in order to be convincing.
For all we know about ROLLUP/CUBE implementation issues right now, doing
this feature with separate implementations might make that easier not
harder. (I note that the crux of my complaint right now is that we're
asking sfuncs to serve two masters --- how's it going to be better when
they have to serve three or four?)
> As for evidence - have you looked at the patch I posted? I'd be very
> interested to know if it removes the performance differences you saw.
(1) You can't really prove the absence of a performance issue by showing
that one specific aggregate doesn't have an issue. (2) These results
(mine as well as yours) seem mighty platform-dependent, so the fact you
don't see an issue doesn't mean someone else won't. (3) A new
FunctionCallInfoData field just for this? Surely not. There's got to be
a distributed cost to that (although I notice you didn't bother
initializing the field most places, which is cheating).
Now point 3 could be addressed by doing the signaling in some other way
with the existing context field. But it's still the case that you're
trying to band-aid a bad design. There's no good reason to make the sfunc
do extra work to be invertible in contexts where we know, with certainty,
that that work is useless. Especially not when we know that even a few
instructions of extra work can be performance-significant.
regards, tom lane
