Re: Aggregate ORDER BY patch

>>>>> "Tom" == Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:

>> Updated version of the aggregate order by patch.

Tom> I'm starting to look at this now. I find it rather bizarre to
Tom> merge both the actual arguments of an aggregate and the optional
Tom> ORDER BY expressions into a single targetlist. It doesn't seem
Tom> like that would be an especially convenient representation to
Tom> work with,

It's extremely convenient, since you need the arguments and the
ordering expressions together in a slot in order to feed them to
tuplesort (in the general case where there is more than one
expression); you need a tupledesc to pass to tuplesort; and there are
existing functions to construct all of these things from the tlist.
Also, you want to merge the argument expressions and ordering
expressions where possible, and this is exactly what the existing
transformSortClause stuff expects.

Tom> and I would also expect there to be a nonzero performance hit
Tom> from the extra TargetEntry expression nodes, even when the
Tom> feature is not in use.

I tested for that and couldn't reliably detect any (certainly <2%
on count(i) on generated data, and under the circumstances I was
testing that's not necessarily outside the error margin).

--
Andrew (irc:RhodiumToad)