On 01/19/2017 04:09 AM, Ashutosh Bapat wrote:
> On Thu, Jan 19, 2017 at 12:02 AM, Robert Haas <robertmhaas@gmail.com> wrote:
>> On Tue, Jan 17, 2017 at 11:33 PM, Ashutosh Bapat
>>
>>> Also, the point is naming that kind of function as aggtransmultifn
>>> would mean that it's always supposed to multiply, which isn't true for
>>> all aggregates.
>>
>> TransValue * integer = newTransValue is well-defined for any
>> aggregate. It's the result of aggregating that TransValue with itself
>> a number of times defined by the integer. And that might well be
>> significantly faster than using aggcombinefn many times. On the other
>> hand, how many queries just sit there are re-aggregate the same
>> TransValues over and over again? I am having trouble wrapping my head
>> around that part of this.
>
> Not all aggregates have TransValue * integer = newTransValue
> behaviour. Example is array_agg() or string_agg() has "TransValue
> concatenated integer time" behaviour. Or an aggregate "multiplying"
> values across rows will have TransValue (raised to) integer behaviour.
> Labelling all of those as "multi" doesn't look good.
>
All aggregates that have (or can have) a combine function have it,
because in the worst case you can simply implement it by calling the
combine function repeatedly.
Also, if you treat the combine function as "+" then the "multiply"
function is exactly what "*" is expected to do. So I find the naming
quite appropriate, actually.
But I think naming of the function is not the most important aspect of
the patch, I believe. In the worst case, we can do s/multi/whatever/
sometime later.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
