Re: Variadic aggregates vs. project policy - Mailing list pgsql-hackers

From Andres Freund
Subject Re: Variadic aggregates vs. project policy
Date
Msg-id 20130829221632.GA5908@awork2.anarazel.de
Whole thread Raw
In response to Variadic aggregates vs. project policy  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Variadic aggregates vs. project policy
List pgsql-hackers
On 2013-08-29 15:55:13 -0400, Tom Lane wrote:
> So I was hacking away at supporting variadic aggregates (per an internal
> request at Salesforce), and had it pretty much working, when I came across
> this old comment in opr_sanity.sql:
> 
> -- Check that there are not aggregates with the same name and different
> -- numbers of arguments.  While not technically wrong, we have a project policy
> -- to avoid this because it opens the door for confusion in connection with
> -- ORDER BY: novices frequently put the ORDER BY in the wrong place.
> -- See the fate of the single-argument form of string_agg() for history.
> -- The only aggregates that should show up here are count(x) and count(*).
> 
> While a variadic-using aggregate doesn't actually trip the associated test
> query, it surely violates the spirit of this policy: if you put ORDER BY
> in the wrong place the parser will be unable to detect that that wasn't
> what you meant.
> 
> For context see the thread starting here:
> http://www.postgresql.org/message-id/AANLkTikV5ok2tS8t6V+gsAPtE3N6TJq1JpPhMZhG2XL0@mail.gmail.com
> In that thread we agreed that this "policy" might be rather squishy,
> but we should at least think hard about whether it would be wise to create
> built-in aggregates with the same name and different numbers of arguments.
> 
> So the question I'm now wondering about is whether this consideration
> makes variadic aggregates a bad idea all around, even if we don't have
> any built-in ones.  Is the risk of user confusion (in the use of their
> own aggregate) sufficient reason to reject such a feature?

I vote for abolishing that policy or maybe weakinging it. As you comment
somewhere downthread the policy just prohibits core functions, but even
for those it looks too strong for me. There are some useful variadic
aggregates I'd like to see and I don't think that the kind of errors
prevented by the policy are frequent enough to warrant a blanket
prohibition.
I'd say we let the check in there but have a list of exceptions in it so
that one has to explicitly think about the issue before adding the
function. Some functions are worth the risk, others are not. E.g. the 1
argument form of string_agg() doesn't have enough benefits, but other
functions might.

Greetings,

Andres Freund

-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services



pgsql-hackers by date:

Previous
From: Ants Aasma
Date:
Subject: Re: Master-slave visibility order
Next
From: Ants Aasma
Date:
Subject: Re: Master-slave visibility order