On Tue, Dec 17, 2013 at 1:18 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Once again: this patch has no business changing any user-visible behavior.
That would include not changing the number of evaluations of volatile functions. The planner is full of places where optimizations are disabled for volatile subexpressions, and I don't see why this should be different.
My point was meant to be more along the lines of that I thought it was already broken and it perhaps should be fixed or at the very least we could warn the users about it.
I would imagine that most of those other places in the planner are to prevent extra evaluations of volatile functions? In this particular case we're already evaluating these multiple extra times when a tuple moves of the top of the frame. I would have thought that we should only evaluate the volatile function once per tuple. This is not what the current implementation does.
I don't have an issue skipping this optimisation when the aggregate's expression contain any volatile functions. I just wanted to raise my concerns about the current behaviour, which I find a bit bizarre.