>>>>> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:
Tom> 1. I really hate the way you've overloaded the transvalue to doTom> something that has approximately nothing to do
withtransitionTom> state (and haven't updated catalogs.sgml to explain that,Tom> either). Seems like it'd be cleaner
tojust hardwire a boolTom> column that distinguishes regular and hypothetical input rows.
The intention here is that while the provided functions all fit the
spec's idea of how inverse distribution or hypothetical set functions
work, the actual implementation mechanisms are more generally
applicable than that and a user-supplied function could well find
something else useful to do with them. Accordingly, hardcoding stuff
seemed inappropriate.
Tom> And why do you need aggtranssortop for that? I fail to see theTom> point of sorting on the flag column.
It is convenient to the implementation to be able to rely on
encountering the hypothetical row deterministically before (or in some
cases after, as in cume_dist) its peers in the remainder of the sort
order. Removing that sort would make the results of the functions
incorrect.
There should probably be some comments about that. oops.
Tom> 2 I also don't care for the definition of aggordnargs, which isTom> the number of direct arguments to an ordered
setfunction,Tom> except when it isn't. Rather than overloading it to be both aTom> count and a couple of flags, I
wonderwhether we shouldn'tTom> expand aggisordsetfunc to be a three-way "aggregate kind" fieldTom> (ordinary agg,
orderedset, or hypothetical set function).
It would still be overloaded in some sense because a non-hypothetical
ordered set function could still take an arbitrary number of args
(using variadic "any") - there aren't any provided, but there's no
good reason to disallow user-defined functions doing that - so you'd
still need a special value like -1 for aggordnargs to handle that.
--
Andrew (irc:RhodiumToad)
