Andrew Gierth <and...@tao11.riddles.org.uk> writes: > Retesting with your changes shows that the gap is down to 15% but still > present:
There's probably some overhead from traversing advance_transition_function for each row, which your version wouldn't have done. 15% sounds pretty high for that though, since advance_transition_function doesn't have much to do when the transition function is non strict and the state value is pass-by-value (which "internal" is). It's possible that we could do something to micro-optimize that case. I also wondered if it was possible to omit rechecking AggCheckCallContext after the first time through in ordered_set_transition(). It seemed unsafe to perhaps not have that happen at all, since if the point is to detect misuse then the mere fact that argument 0 isn't null isn't much comfort. It strikes me though that now we could test for "first call" by looking at fcinfo->flinfo->fn_extra, and be pretty sure that we've already checked the context if that isn't null. Whether this would save anything noticeable isn't clear though; I didn't see AggCheckCallContext high in the profile. > Furthermore, I can't help noticing that the increased complexity has > now pretty much negated your original arguments for moving so much of > the work out of nodeAgg.c. The key reason for that was, and remains, not having the behavior hard-wired in nodeAgg; I believe that this design permits things to be accomplished in aggregate implementation functions that would not have been possible with the original patch. I'm willing to accept some code growth to have that flexibility. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers