On ons, 2012-04-04 at 18:59 -0400, Tom Lane wrote: > > >> Why not call it string_agg? > > > Here is a patch to do the renaming. As it stands, it fails the > > opr_sanity regression test, because that complains that there are now > > two aggregate functions string_agg with different number of arguments. > > It seems to me that that test should really only complain if the common > > argument types of the two aggregates are the same, correct? > > Uh, no. That test is there for good and sufficient reasons, as per its > comment: > > -- 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 renaming you propose would only be acceptable to those who have > forgotten that history. I haven't.
I had reviewed that thread very carefully, but I'm not sure it applies. The issue was that we don't want aggregates with optional second argument, because "novices" could confuse agg(a, b ORDER BY c) with agg(a ORDER BY b, c) -- wrong without the error being flagged. But that doesn't apply if the first argument has different types. Erroneously calling agg(textdatum ORDER BY textdatum, sthelse) will not result in a call to agg(bytea). (Unless the textdatum is really an unknown literal, but that would be silly.) Nevertheless, the problem would now be that adding string_agg(bytea) would effectively forbid adding string_agg(bytea, delim) in the future. So making a two-argument string_agg(bytea, bytea) now seems like the best solution anyway. (This applies independently of the function renaming, actually.) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers