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.
This commit is contained in:
Tom Lane
parent
fd1843ff89
commit
b0c451e145
|
|
@ -1,4 +1,4 @@
|
|||
<!-- $PostgreSQL: pgsql/doc/src/sgml/func.sgml,v 1.523 2010/08/05 04:21:53 petere Exp $ -->
|
||||
<!-- $PostgreSQL: pgsql/doc/src/sgml/func.sgml,v 1.524 2010/08/05 18:21:17 tgl Exp $ -->
|
||||
|
||||
<chapter id="functions">
|
||||
<title>Functions and Operators</title>
|
||||
|
|
@ -9782,7 +9782,7 @@ SELECT NULLIF(value, '(none)') ...
|
|||
<thead>
|
||||
<row>
|
||||
<entry>Function</entry>
|
||||
<entry>Argument Type</entry>
|
||||
<entry>Argument Type(s)</entry>
|
||||
<entry>Return Type</entry>
|
||||
<entry>Description</entry>
|
||||
</row>
|
||||
|
|
@ -9952,17 +9952,17 @@ SELECT NULLIF(value, '(none)') ...
|
|||
<primary>string_agg</primary>
|
||||
</indexterm>
|
||||
<function>
|
||||
string_agg(<replaceable class="parameter">expression</replaceable>
|
||||
[, <replaceable class="parameter">delimiter</replaceable> ] )
|
||||
string_agg(<replaceable class="parameter">expression</replaceable>,
|
||||
<replaceable class="parameter">delimiter</replaceable>)
|
||||
</function>
|
||||
</entry>
|
||||
<entry>
|
||||
<type>text</type>
|
||||
<type>text</type>, <type>text</type>
|
||||
</entry>
|
||||
<entry>
|
||||
<type>text</type>
|
||||
</entry>
|
||||
<entry>input values concatenated into a string, optionally with delimiters</entry>
|
||||
<entry>input values concatenated into a string, separated by delimiter</entry>
|
||||
</row>
|
||||
|
||||
<row>
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
<!-- $PostgreSQL: pgsql/doc/src/sgml/syntax.sgml,v 1.149 2010/08/04 15:27:57 tgl Exp $ -->
|
||||
<!-- $PostgreSQL: pgsql/doc/src/sgml/syntax.sgml,v 1.150 2010/08/05 18:21:17 tgl Exp $ -->
|
||||
|
||||
<chapter id="sql-syntax">
|
||||
<title>SQL Syntax</title>
|
||||
|
|
@ -1583,16 +1583,17 @@ SELECT array_agg(a ORDER BY b DESC) FROM table;
|
|||
<para>
|
||||
When dealing with multiple-argument aggregate functions, note that the
|
||||
<literal>ORDER BY</> clause goes after all the aggregate arguments.
|
||||
For example, this:
|
||||
For example, write this:
|
||||
<programlisting>
|
||||
SELECT string_agg(a, ',' ORDER BY a) FROM table;
|
||||
</programlisting>
|
||||
not this:
|
||||
<programlisting>
|
||||
SELECT string_agg(a ORDER BY a, ',') FROM table; -- not what you want
|
||||
SELECT string_agg(a ORDER BY a, ',') FROM table; -- incorrect
|
||||
</programlisting>
|
||||
The latter syntax will be accepted, but <literal>','</> will be
|
||||
treated as a (useless) sort key.
|
||||
The latter is syntactically valid, but it represents a call of a
|
||||
single-argument aggregate function with two <literal>ORDER BY</> keys
|
||||
(the second one being rather useless since it's a constant).
|
||||
</para>
|
||||
|
||||
<para>
|
||||
|
|
|
|||
|
|
@ -8,7 +8,7 @@
|
|||
*
|
||||
*
|
||||
* IDENTIFICATION
|
||||
* $PostgreSQL: pgsql/src/backend/utils/adt/varlena.c,v 1.177 2010/02/26 02:01:10 momjian Exp $
|
||||
* $PostgreSQL: pgsql/src/backend/utils/adt/varlena.c,v 1.178 2010/08/05 18:21:17 tgl Exp $
|
||||
*
|
||||
*-------------------------------------------------------------------------
|
||||
*/
|
||||
|
|
@ -3320,7 +3320,7 @@ pg_column_size(PG_FUNCTION_ARGS)
|
|||
/*
|
||||
* string_agg - Concatenates values and returns string.
|
||||
*
|
||||
* Syntax: string_agg(value text, delimiter text = '') RETURNS text
|
||||
* Syntax: string_agg(value text, delimiter text) RETURNS text
|
||||
*
|
||||
* Note: Any NULL values are ignored. The first-call delimiter isn't
|
||||
* actually used at all, and on subsequent calls the delimiter precedes
|
||||
|
|
@ -3359,28 +3359,6 @@ string_agg_transfn(PG_FUNCTION_ARGS)
|
|||
|
||||
state = PG_ARGISNULL(0) ? NULL : (StringInfo) PG_GETARG_POINTER(0);
|
||||
|
||||
/* Append the element unless null. */
|
||||
if (!PG_ARGISNULL(1))
|
||||
{
|
||||
if (state == NULL)
|
||||
state = makeStringAggState(fcinfo);
|
||||
appendStringInfoText(state, PG_GETARG_TEXT_PP(1)); /* value */
|
||||
}
|
||||
|
||||
/*
|
||||
* The transition type for string_agg() is declared to be "internal",
|
||||
* which is a pass-by-value type the same size as a pointer.
|
||||
*/
|
||||
PG_RETURN_POINTER(state);
|
||||
}
|
||||
|
||||
Datum
|
||||
string_agg_delim_transfn(PG_FUNCTION_ARGS)
|
||||
{
|
||||
StringInfo state;
|
||||
|
||||
state = PG_ARGISNULL(0) ? NULL : (StringInfo) PG_GETARG_POINTER(0);
|
||||
|
||||
/* Append the value unless null. */
|
||||
if (!PG_ARGISNULL(1))
|
||||
{
|
||||
|
|
|
|||
|
|
@ -37,7 +37,7 @@
|
|||
* Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
|
||||
* Portions Copyright (c) 1994, Regents of the University of California
|
||||
*
|
||||
* $PostgreSQL: pgsql/src/include/catalog/catversion.h,v 1.589 2010/08/05 04:21:54 petere Exp $
|
||||
* $PostgreSQL: pgsql/src/include/catalog/catversion.h,v 1.590 2010/08/05 18:21:17 tgl Exp $
|
||||
*
|
||||
*-------------------------------------------------------------------------
|
||||
*/
|
||||
|
|
@ -53,6 +53,6 @@
|
|||
*/
|
||||
|
||||
/* yyyymmddN */
|
||||
#define CATALOG_VERSION_NO 201008051
|
||||
#define CATALOG_VERSION_NO 201008052
|
||||
|
||||
#endif
|
||||
|
|
|
|||
|
|
@ -8,7 +8,7 @@
|
|||
* Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
|
||||
* Portions Copyright (c) 1994, Regents of the University of California
|
||||
*
|
||||
* $PostgreSQL: pgsql/src/include/catalog/pg_aggregate.h,v 1.71 2010/02/01 03:14:43 itagaki Exp $
|
||||
* $PostgreSQL: pgsql/src/include/catalog/pg_aggregate.h,v 1.72 2010/08/05 18:21:17 tgl Exp $
|
||||
*
|
||||
* NOTES
|
||||
* the genbki.pl script reads this file and generates .bki
|
||||
|
|
@ -224,8 +224,7 @@ DATA(insert ( 2901 xmlconcat2 - 0 142 _null_ ));
|
|||
DATA(insert ( 2335 array_agg_transfn array_agg_finalfn 0 2281 _null_ ));
|
||||
|
||||
/* text */
|
||||
DATA(insert (3537 string_agg_transfn string_agg_finalfn 0 2281 _null_ ));
|
||||
DATA(insert (3538 string_agg_delim_transfn string_agg_finalfn 0 2281 _null_ ));
|
||||
DATA(insert ( 3538 string_agg_transfn string_agg_finalfn 0 2281 _null_ ));
|
||||
|
||||
/*
|
||||
* prototypes for functions in pg_aggregate.c
|
||||
|
|
|
|||
|
|
@ -7,7 +7,7 @@
|
|||
* Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
|
||||
* Portions Copyright (c) 1994, Regents of the University of California
|
||||
*
|
||||
* $PostgreSQL: pgsql/src/include/catalog/pg_proc.h,v 1.574 2010/08/05 04:21:54 petere Exp $
|
||||
* $PostgreSQL: pgsql/src/include/catalog/pg_proc.h,v 1.575 2010/08/05 18:21:17 tgl Exp $
|
||||
*
|
||||
* NOTES
|
||||
* The script catalog/genbki.pl reads this file and generates .bki
|
||||
|
|
@ -2835,16 +2835,12 @@ DATA(insert OID = 2816 ( float8_covar_samp PGNSP PGUID 12 1 0 0 f f f t f i 1
|
|||
DESCR("COVAR_SAMP(double, double) aggregate final function");
|
||||
DATA(insert OID = 2817 ( float8_corr PGNSP PGUID 12 1 0 0 f f f t f i 1 0 701 "1022" _null_ _null_ _null_ _null_ float8_corr _null_ _null_ _null_ ));
|
||||
DESCR("CORR(double, double) aggregate final function");
|
||||
DATA(insert OID = 3534 ( string_agg_transfn PGNSP PGUID 12 1 0 0 f f f f f i 2 0 2281 "2281 25" _null_ _null_ _null_ _null_ string_agg_transfn _null_ _null_ _null_ ));
|
||||
DESCR("string_agg(text) transition function");
|
||||
DATA(insert OID = 3535 ( string_agg_delim_transfn PGNSP PGUID 12 1 0 0 f f f f f i 3 0 2281 "2281 25 25" _null_ _null_ _null_ _null_ string_agg_delim_transfn _null_ _null_ _null_ ));
|
||||
DATA(insert OID = 3535 ( string_agg_transfn PGNSP PGUID 12 1 0 0 f f f f f i 3 0 2281 "2281 25 25" _null_ _null_ _null_ _null_ string_agg_transfn _null_ _null_ _null_ ));
|
||||
DESCR("string_agg(text, text) transition function");
|
||||
DATA(insert OID = 3536 ( string_agg_finalfn PGNSP PGUID 12 1 0 0 f f f f f i 1 0 25 "2281" _null_ _null_ _null_ _null_ string_agg_finalfn _null_ _null_ _null_ ));
|
||||
DESCR("string_agg final function");
|
||||
DATA(insert OID = 3537 ( string_agg PGNSP PGUID 12 1 0 0 t f f f f i 1 0 25 "25" _null_ _null_ _null_ _null_ aggregate_dummy _null_ _null_ _null_ ));
|
||||
DESCR("concatenate aggregate input into an string");
|
||||
DESCR("string_agg(text, text) final function");
|
||||
DATA(insert OID = 3538 ( string_agg PGNSP PGUID 12 1 0 0 t f f f f i 2 0 25 "25 25" _null_ _null_ _null_ _null_ aggregate_dummy _null_ _null_ _null_ ));
|
||||
DESCR("concatenate aggregate input into an string with delimiter");
|
||||
DESCR("concatenate aggregate input into a string");
|
||||
|
||||
/* To ASCII conversion */
|
||||
DATA(insert OID = 1845 ( to_ascii PGNSP PGUID 12 1 0 0 f f f t f i 1 0 25 "25" _null_ _null_ _null_ _null_ to_ascii_default _null_ _null_ _null_ ));
|
||||
|
|
|
|||
|
|
@ -7,7 +7,7 @@
|
|||
* Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
|
||||
* Portions Copyright (c) 1994, Regents of the University of California
|
||||
*
|
||||
* $PostgreSQL: pgsql/src/include/utils/builtins.h,v 1.352 2010/07/22 01:22:35 rhaas Exp $
|
||||
* $PostgreSQL: pgsql/src/include/utils/builtins.h,v 1.353 2010/08/05 18:21:19 tgl Exp $
|
||||
*
|
||||
*-------------------------------------------------------------------------
|
||||
*/
|
||||
|
|
@ -729,7 +729,6 @@ extern Datum unknownsend(PG_FUNCTION_ARGS);
|
|||
extern Datum pg_column_size(PG_FUNCTION_ARGS);
|
||||
|
||||
extern Datum string_agg_transfn(PG_FUNCTION_ARGS);
|
||||
extern Datum string_agg_delim_transfn(PG_FUNCTION_ARGS);
|
||||
extern Datum string_agg_finalfn(PG_FUNCTION_ARGS);
|
||||
|
||||
/* version.c */
|
||||
|
|
@ -780,9 +779,6 @@ extern Datum translate(PG_FUNCTION_ARGS);
|
|||
extern Datum chr (PG_FUNCTION_ARGS);
|
||||
extern Datum repeat(PG_FUNCTION_ARGS);
|
||||
extern Datum ascii(PG_FUNCTION_ARGS);
|
||||
extern Datum string_agg_transfn(PG_FUNCTION_ARGS);
|
||||
extern Datum string_agg_delim_transfn(PG_FUNCTION_ARGS);
|
||||
extern Datum string_agg_finalfn(PG_FUNCTION_ARGS);
|
||||
|
||||
/* inet_net_ntop.c */
|
||||
extern char *inet_net_ntop(int af, const void *src, int bits,
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Reference in New Issue