Remove the single-argument form of string_agg(). It added nothing much in

functionality, while creating an ambiguity in usage with ORDER BY that at
least two people have already gotten seriously confused by.  Also, add an
opr_sanity test to check that we don't in future violate the newly minted
policy of not having built-in aggregates with the same name and different
numbers of parameters.  Per discussion of a complaint from Thom Brown.

4 files changed, 62 insertions, 24 deletions

diff --git a/src/test/regress/expected/aggregates.out b/src/test/regress/expected/aggregates.out
index 087b4679d4d..b456d7e989a 100644
--- a/src/test/regress/expected/aggregates.out
+++ b/src/test/regress/expected/aggregates.out
@@ -800,12 +800,6 @@ ERROR:  in an aggregate with DISTINCT, ORDER BY expressions must appear in argum
 LINE 1: select aggfns(distinct a,a,c order by a,b)
                                                 ^
 -- string_agg tests
-select string_agg(a) from (values('aaaa'),('bbbb'),('cccc')) g(a);
-  string_agg  
---------------
- aaaabbbbcccc
-(1 row)
-
 select string_agg(a,',') from (values('aaaa'),('bbbb'),('cccc')) g(a);
    string_agg   
 ----------------
@@ -818,10 +812,10 @@ select string_agg(a,',') from (values('aaaa'),(null),('bbbb'),('cccc')) g(a);
  aaaa,bbbb,cccc
 (1 row)
 
-select string_agg(a,',') from (values(null),(null),('bbbb'),('cccc')) g(a);
+select string_agg(a,'AB') from (values(null),(null),('bbbb'),('cccc')) g(a);
  string_agg 
 ------------
- bbbb,cccc
+ bbbbABcccc
 (1 row)
 
 select string_agg(a,',') from (values(null),(null)) g(a);
@@ -831,23 +825,23 @@ select string_agg(a,',') from (values(null),(null)) g(a);
 (1 row)
 
 -- check some implicit casting cases, as per bug #5564
-select string_agg(distinct f1 order by f1) from varchar_tbl;  -- ok
+select string_agg(distinct f1, ',' order by f1) from varchar_tbl;  -- ok
  string_agg 
 ------------
- aababcd
+ a,ab,abcd
 (1 row)
 
-select string_agg(distinct f1::text order by f1) from varchar_tbl;  -- not ok
+select string_agg(distinct f1::text, ',' order by f1) from varchar_tbl;  -- not ok
 ERROR:  in an aggregate with DISTINCT, ORDER BY expressions must appear in argument list
-LINE 1: select string_agg(distinct f1::text order by f1) from varcha...
-                                                     ^
-select string_agg(distinct f1 order by f1::text) from varchar_tbl;  -- not ok
+LINE 1: select string_agg(distinct f1::text, ',' order by f1) from v...
+                                                          ^
+select string_agg(distinct f1, ',' order by f1::text) from varchar_tbl;  -- not ok
 ERROR:  in an aggregate with DISTINCT, ORDER BY expressions must appear in argument list
-LINE 1: select string_agg(distinct f1 order by f1::text) from varcha...
-                                               ^
-select string_agg(distinct f1::text order by f1::text) from varchar_tbl;  -- ok
+LINE 1: select string_agg(distinct f1, ',' order by f1::text) from v...
+                                                    ^
+select string_agg(distinct f1::text, ',' order by f1::text) from varchar_tbl;  -- ok
  string_agg 
 ------------
- aababcd
+ a,ab,abcd
 (1 row)
 
diff --git a/src/test/regress/expected/opr_sanity.out b/src/test/regress/expected/opr_sanity.out
index 5e36d481dfb..f6fee25de55 100644
--- a/src/test/regress/expected/opr_sanity.out
+++ b/src/test/regress/expected/opr_sanity.out
@@ -773,6 +773,31 @@ ORDER BY 1, 2;
  min     | <       |            1
 (2 rows)
 
+-- 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(*).
+SELECT p1.oid::regprocedure, p2.oid::regprocedure
+FROM pg_proc AS p1, pg_proc AS p2
+WHERE p1.oid < p2.oid AND p1.proname = p2.proname AND
+    p1.proisagg AND p2.proisagg AND
+    array_dims(p1.proargtypes) != array_dims(p2.proargtypes)
+ORDER BY 1;
+     oid      |   oid   
+--------------+---------
+ count("any") | count()
+(1 row)
+
+-- For the same reason, aggregates with default arguments are no good.
+SELECT oid, proname
+FROM pg_proc AS p
+WHERE proisagg AND proargdefaults IS NOT NULL;
+ oid | proname 
+-----+---------
+(0 rows)
+
 -- **************** pg_opfamily ****************
 -- Look for illegal values in pg_opfamily fields
 SELECT p1.oid
diff --git a/src/test/regress/sql/aggregates.sql b/src/test/regress/sql/aggregates.sql
index b2199d1ce94..8f81ba763a0 100644
--- a/src/test/regress/sql/aggregates.sql
+++ b/src/test/regress/sql/aggregates.sql
@@ -357,14 +357,13 @@ select aggfns(distinct a,a,c order by a,b)
   from (values (1,1,'foo')) v(a,b,c), generate_series(1,2) i;
 
 -- string_agg tests
-select string_agg(a) from (values('aaaa'),('bbbb'),('cccc')) g(a);
 select string_agg(a,',') from (values('aaaa'),('bbbb'),('cccc')) g(a);
 select string_agg(a,',') from (values('aaaa'),(null),('bbbb'),('cccc')) g(a);
-select string_agg(a,',') from (values(null),(null),('bbbb'),('cccc')) g(a);
+select string_agg(a,'AB') from (values(null),(null),('bbbb'),('cccc')) g(a);
 select string_agg(a,',') from (values(null),(null)) g(a);
 
 -- check some implicit casting cases, as per bug #5564
-select string_agg(distinct f1 order by f1) from varchar_tbl;  -- ok
-select string_agg(distinct f1::text order by f1) from varchar_tbl;  -- not ok
-select string_agg(distinct f1 order by f1::text) from varchar_tbl;  -- not ok
-select string_agg(distinct f1::text order by f1::text) from varchar_tbl;  -- ok
+select string_agg(distinct f1, ',' order by f1) from varchar_tbl;  -- ok
+select string_agg(distinct f1::text, ',' order by f1) from varchar_tbl;  -- not ok
+select string_agg(distinct f1, ',' order by f1::text) from varchar_tbl;  -- not ok
+select string_agg(distinct f1::text, ',' order by f1::text) from varchar_tbl;  -- ok
diff --git a/src/test/regress/sql/opr_sanity.sql b/src/test/regress/sql/opr_sanity.sql
index 38866c9a549..46ec24cca6f 100644
--- a/src/test/regress/sql/opr_sanity.sql
+++ b/src/test/regress/sql/opr_sanity.sql
@@ -621,6 +621,26 @@ WHERE a.aggfnoid = p.oid AND a.aggsortop = o.oid AND
     amopmethod = (SELECT oid FROM pg_am WHERE amname = 'btree')
 ORDER BY 1, 2;
 
+-- 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(*).
+
+SELECT p1.oid::regprocedure, p2.oid::regprocedure
+FROM pg_proc AS p1, pg_proc AS p2
+WHERE p1.oid < p2.oid AND p1.proname = p2.proname AND
+    p1.proisagg AND p2.proisagg AND
+    array_dims(p1.proargtypes) != array_dims(p2.proargtypes)
+ORDER BY 1;
+
+-- For the same reason, aggregates with default arguments are no good.
+
+SELECT oid, proname
+FROM pg_proc AS p
+WHERE proisagg AND proargdefaults IS NOT NULL;
+
 -- **************** pg_opfamily ****************
 
 -- Look for illegal values in pg_opfamily fields