Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitb0c451e

Browse files
committed
Remove the single-argument form of string_agg(). It added nothing much in
functionality, while creating an ambiguity in usage with ORDER BY that atleast two people have already gotten seriously confused by. Also, add anopr_sanity test to check that we don't in future violate the newly mintedpolicy of not having built-in aggregates with the same name and differentnumbers of parameters. Per discussion of a complaint from Thom Brown.
1 parentfd1843f commitb0c451e

File tree

11 files changed

+85
-77
lines changed

11 files changed

+85
-77
lines changed

‎doc/src/sgml/func.sgml

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<!-- $PostgreSQL: pgsql/doc/src/sgml/func.sgml,v 1.523 2010/08/0504:21:53 petere Exp $ -->
1+
<!-- $PostgreSQL: pgsql/doc/src/sgml/func.sgml,v 1.524 2010/08/0518:21:17 tgl Exp $ -->
22

33
<chapter id="functions">
44
<title>Functions and Operators</title>
@@ -9782,7 +9782,7 @@ SELECT NULLIF(value, '(none)') ...
97829782
<thead>
97839783
<row>
97849784
<entry>Function</entry>
9785-
<entry>Argument Type</entry>
9785+
<entry>Argument Type(s)</entry>
97869786
<entry>Return Type</entry>
97879787
<entry>Description</entry>
97889788
</row>
@@ -9952,17 +9952,17 @@ SELECT NULLIF(value, '(none)') ...
99529952
<primary>string_agg</primary>
99539953
</indexterm>
99549954
<function>
9955-
string_agg(<replaceable class="parameter">expression</replaceable>
9956-
[,<replaceable class="parameter">delimiter</replaceable> ])
9955+
string_agg(<replaceable class="parameter">expression</replaceable>,
9956+
<replaceable class="parameter">delimiter</replaceable>)
99579957
</function>
99589958
</entry>
99599959
<entry>
9960-
<type>text</type>
9960+
<type>text</type>, <type>text</type>
99619961
</entry>
99629962
<entry>
99639963
<type>text</type>
99649964
</entry>
9965-
<entry>input values concatenated into a string,optionally with delimiters</entry>
9965+
<entry>input values concatenated into a string,separated by delimiter</entry>
99669966
</row>
99679967

99689968
<row>

‎doc/src/sgml/syntax.sgml

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<!-- $PostgreSQL: pgsql/doc/src/sgml/syntax.sgml,v 1.149 2010/08/04 15:27:57 tgl Exp $ -->
1+
<!-- $PostgreSQL: pgsql/doc/src/sgml/syntax.sgml,v 1.150 2010/08/05 18:21:17 tgl Exp $ -->
22

33
<chapter id="sql-syntax">
44
<title>SQL Syntax</title>
@@ -1583,16 +1583,17 @@ SELECT array_agg(a ORDER BY b DESC) FROM table;
15831583
<para>
15841584
When dealing with multiple-argument aggregate functions, note that the
15851585
<literal>ORDER BY</> clause goes after all the aggregate arguments.
1586-
For example, this:
1586+
For example,writethis:
15871587
<programlisting>
15881588
SELECT string_agg(a, ',' ORDER BY a) FROM table;
15891589
</programlisting>
15901590
not this:
15911591
<programlisting>
1592-
SELECT string_agg(a ORDER BY a, ',') FROM table; --not what you want
1592+
SELECT string_agg(a ORDER BY a, ',') FROM table; --incorrect
15931593
</programlisting>
1594-
The latter syntax will be accepted, but <literal>','</> will be
1595-
treated as a (useless) sort key.
1594+
The latter is syntactically valid, but it represents a call of a
1595+
single-argument aggregate function with two <literal>ORDER BY</> keys
1596+
(the second one being rather useless since it's a constant).
15961597
</para>
15971598

15981599
<para>

‎src/backend/utils/adt/varlena.c

Lines changed: 2 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/utils/adt/varlena.c,v 1.177 2010/02/26 02:01:10 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/utils/adt/varlena.c,v 1.178 2010/08/05 18:21:17 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -3320,7 +3320,7 @@ pg_column_size(PG_FUNCTION_ARGS)
33203320
/*
33213321
* string_agg - Concatenates values and returns string.
33223322
*
3323-
* Syntax: string_agg(value text, delimiter text = '') RETURNS text
3323+
* Syntax: string_agg(value text, delimiter text) RETURNS text
33243324
*
33253325
* Note: Any NULL values are ignored. The first-call delimiter isn't
33263326
* actually used at all, and on subsequent calls the delimiter precedes
@@ -3359,28 +3359,6 @@ string_agg_transfn(PG_FUNCTION_ARGS)
33593359

33603360
state=PG_ARGISNULL(0) ?NULL : (StringInfo)PG_GETARG_POINTER(0);
33613361

3362-
/* Append the element unless null. */
3363-
if (!PG_ARGISNULL(1))
3364-
{
3365-
if (state==NULL)
3366-
state=makeStringAggState(fcinfo);
3367-
appendStringInfoText(state,PG_GETARG_TEXT_PP(1));/* value */
3368-
}
3369-
3370-
/*
3371-
* The transition type for string_agg() is declared to be "internal",
3372-
* which is a pass-by-value type the same size as a pointer.
3373-
*/
3374-
PG_RETURN_POINTER(state);
3375-
}
3376-
3377-
Datum
3378-
string_agg_delim_transfn(PG_FUNCTION_ARGS)
3379-
{
3380-
StringInfostate;
3381-
3382-
state=PG_ARGISNULL(0) ?NULL : (StringInfo)PG_GETARG_POINTER(0);
3383-
33843362
/* Append the value unless null. */
33853363
if (!PG_ARGISNULL(1))
33863364
{

‎src/include/catalog/catversion.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
* Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
3838
* Portions Copyright (c) 1994, Regents of the University of California
3939
*
40-
* $PostgreSQL: pgsql/src/include/catalog/catversion.h,v 1.589 2010/08/0504:21:54 petere Exp $
40+
* $PostgreSQL: pgsql/src/include/catalog/catversion.h,v 1.590 2010/08/0518:21:17 tgl Exp $
4141
*
4242
*-------------------------------------------------------------------------
4343
*/
@@ -53,6 +53,6 @@
5353
*/
5454

5555
/*yyyymmddN */
56-
#defineCATALOG_VERSION_NO201008051
56+
#defineCATALOG_VERSION_NO201008052
5757

5858
#endif

‎src/include/catalog/pg_aggregate.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
* Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
99
* Portions Copyright (c) 1994, Regents of the University of California
1010
*
11-
* $PostgreSQL: pgsql/src/include/catalog/pg_aggregate.h,v 1.71 2010/02/01 03:14:43 itagaki Exp $
11+
* $PostgreSQL: pgsql/src/include/catalog/pg_aggregate.h,v 1.72 2010/08/05 18:21:17 tgl Exp $
1212
*
1313
* NOTES
1414
* the genbki.pl script reads this file and generates .bki
@@ -224,8 +224,7 @@ DATA(insert ( 2901 xmlconcat2 -0142_null_ ));
224224
DATA(insert (2335array_agg_transfnarray_agg_finalfn02281_null_ ));
225225

226226
/* text */
227-
DATA(insert (3537string_agg_transfnstring_agg_finalfn02281_null_ ));
228-
DATA(insert (3538string_agg_delim_transfnstring_agg_finalfn02281_null_ ));
227+
DATA(insert (3538string_agg_transfnstring_agg_finalfn02281_null_ ));
229228

230229
/*
231230
* prototypes for functions in pg_aggregate.c

‎src/include/catalog/pg_proc.h

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $PostgreSQL: pgsql/src/include/catalog/pg_proc.h,v 1.574 2010/08/0504:21:54 petere Exp $
10+
* $PostgreSQL: pgsql/src/include/catalog/pg_proc.h,v 1.575 2010/08/0518:21:17 tgl Exp $
1111
*
1212
* NOTES
1313
* The script catalog/genbki.pl reads this file and generates .bki
@@ -2835,16 +2835,12 @@ DATA(insert OID = 2816 ( float8_covar_sampPGNSP PGUID 12 1 0 0 f f f t f i 1
28352835
DESCR("COVAR_SAMP(double, double) aggregate final function");
28362836
DATA(insertOID=2817 (float8_corrPGNSPPGUID12100ffftfi10701"1022"_null__null__null__null_float8_corr_null__null__null_ ));
28372837
DESCR("CORR(double, double) aggregate final function");
2838-
DATA(insertOID=3534 (string_agg_transfnPGNSPPGUID12100fffffi202281"2281 25"_null__null__null__null_string_agg_transfn_null__null__null_ ));
2839-
DESCR("string_agg(text) transition function");
2840-
DATA(insertOID=3535 (string_agg_delim_transfnPGNSPPGUID12100fffffi302281"2281 25 25"_null__null__null__null_string_agg_delim_transfn_null__null__null_ ));
2838+
DATA(insertOID=3535 (string_agg_transfnPGNSPPGUID12100fffffi302281"2281 25 25"_null__null__null__null_string_agg_transfn_null__null__null_ ));
28412839
DESCR("string_agg(text, text) transition function");
28422840
DATA(insertOID=3536 (string_agg_finalfnPGNSPPGUID12100fffffi1025"2281"_null__null__null__null_string_agg_finalfn_null__null__null_ ));
2843-
DESCR("string_agg final function");
2844-
DATA(insertOID=3537 (string_aggPGNSPPGUID12100tffffi1025"25"_null__null__null__null_aggregate_dummy_null__null__null_ ));
2845-
DESCR("concatenate aggregate input into an string");
2841+
DESCR("string_agg(text, text) final function");
28462842
DATA(insertOID=3538 (string_aggPGNSPPGUID12100tffffi2025"25 25"_null__null__null__null_aggregate_dummy_null__null__null_ ));
2847-
DESCR("concatenate aggregate input intoan string with delimiter");
2843+
DESCR("concatenate aggregate input intoa string");
28482844

28492845
/* To ASCII conversion */
28502846
DATA(insertOID=1845 (to_asciiPGNSPPGUID12100ffftfi1025"25"_null__null__null__null_to_ascii_default_null__null__null_ ));

‎src/include/utils/builtins.h

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $PostgreSQL: pgsql/src/include/utils/builtins.h,v 1.352 2010/07/22 01:22:35 rhaas Exp $
10+
* $PostgreSQL: pgsql/src/include/utils/builtins.h,v 1.353 2010/08/05 18:21:19 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -729,7 +729,6 @@ extern Datum unknownsend(PG_FUNCTION_ARGS);
729729
externDatumpg_column_size(PG_FUNCTION_ARGS);
730730

731731
externDatumstring_agg_transfn(PG_FUNCTION_ARGS);
732-
externDatumstring_agg_delim_transfn(PG_FUNCTION_ARGS);
733732
externDatumstring_agg_finalfn(PG_FUNCTION_ARGS);
734733

735734
/* version.c */
@@ -780,9 +779,6 @@ extern Datum translate(PG_FUNCTION_ARGS);
780779
externDatumchr (PG_FUNCTION_ARGS);
781780
externDatumrepeat(PG_FUNCTION_ARGS);
782781
externDatumascii(PG_FUNCTION_ARGS);
783-
externDatumstring_agg_transfn(PG_FUNCTION_ARGS);
784-
externDatumstring_agg_delim_transfn(PG_FUNCTION_ARGS);
785-
externDatumstring_agg_finalfn(PG_FUNCTION_ARGS);
786782

787783
/* inet_net_ntop.c */
788784
externchar*inet_net_ntop(intaf,constvoid*src,intbits,

‎src/test/regress/expected/aggregates.out

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -800,12 +800,6 @@ ERROR: in an aggregate with DISTINCT, ORDER BY expressions must appear in argum
800800
LINE 1: select aggfns(distinct a,a,c order by a,b)
801801
^
802802
-- string_agg tests
803-
select string_agg(a) from (values('aaaa'),('bbbb'),('cccc')) g(a);
804-
string_agg
805-
--------------
806-
aaaabbbbcccc
807-
(1 row)
808-
809803
select string_agg(a,',') from (values('aaaa'),('bbbb'),('cccc')) g(a);
810804
string_agg
811805
----------------
@@ -818,10 +812,10 @@ select string_agg(a,',') from (values('aaaa'),(null),('bbbb'),('cccc')) g(a);
818812
aaaa,bbbb,cccc
819813
(1 row)
820814

821-
select string_agg(a,',') from (values(null),(null),('bbbb'),('cccc')) g(a);
815+
select string_agg(a,'AB') from (values(null),(null),('bbbb'),('cccc')) g(a);
822816
string_agg
823817
------------
824-
bbbb,cccc
818+
bbbbABcccc
825819
(1 row)
826820

827821
select string_agg(a,',') from (values(null),(null)) g(a);
@@ -831,23 +825,23 @@ select string_agg(a,',') from (values(null),(null)) g(a);
831825
(1 row)
832826

833827
-- check some implicit casting cases, as per bug #5564
834-
select string_agg(distinct f1 order by f1) from varchar_tbl; -- ok
828+
select string_agg(distinct f1, ',' order by f1) from varchar_tbl; -- ok
835829
string_agg
836830
------------
837-
aababcd
831+
a,ab,abcd
838832
(1 row)
839833

840-
select string_agg(distinct f1::text order by f1) from varchar_tbl; -- not ok
834+
select string_agg(distinct f1::text, ',' order by f1) from varchar_tbl; -- not ok
841835
ERROR: in an aggregate with DISTINCT, ORDER BY expressions must appear in argument list
842-
LINE 1: select string_agg(distinct f1::textorder by f1) fromvarcha...
843-
^
844-
select string_agg(distinct f1 order by f1::text) from varchar_tbl; -- not ok
836+
LINE 1: select string_agg(distinct f1::text, ','order by f1) fromv...
837+
^
838+
select string_agg(distinct f1, ',' order by f1::text) from varchar_tbl; -- not ok
845839
ERROR: in an aggregate with DISTINCT, ORDER BY expressions must appear in argument list
846-
LINE 1: select string_agg(distinct f1order by f1::text) fromvarcha...
847-
^
848-
select string_agg(distinct f1::text order by f1::text) from varchar_tbl; -- ok
840+
LINE 1: select string_agg(distinct f1, ','order by f1::text) fromv...
841+
^
842+
select string_agg(distinct f1::text, ',' order by f1::text) from varchar_tbl; -- ok
849843
string_agg
850844
------------
851-
aababcd
845+
a,ab,abcd
852846
(1 row)
853847

‎src/test/regress/expected/opr_sanity.out

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -773,6 +773,31 @@ ORDER BY 1, 2;
773773
min | < | 1
774774
(2 rows)
775775

776+
-- Check that there are not aggregates with the same name and different
777+
-- numbers of arguments. While not technically wrong, we have a project policy
778+
-- to avoid this because it opens the door for confusion in connection with
779+
-- ORDER BY: novices frequently put the ORDER BY in the wrong place.
780+
-- See the fate of the single-argument form of string_agg() for history.
781+
-- The only aggregates that should show up here are count(x) and count(*).
782+
SELECT p1.oid::regprocedure, p2.oid::regprocedure
783+
FROM pg_proc AS p1, pg_proc AS p2
784+
WHERE p1.oid < p2.oid AND p1.proname = p2.proname AND
785+
p1.proisagg AND p2.proisagg AND
786+
array_dims(p1.proargtypes) != array_dims(p2.proargtypes)
787+
ORDER BY 1;
788+
oid | oid
789+
--------------+---------
790+
count("any") | count()
791+
(1 row)
792+
793+
-- For the same reason, aggregates with default arguments are no good.
794+
SELECT oid, proname
795+
FROM pg_proc AS p
796+
WHERE proisagg AND proargdefaults IS NOT NULL;
797+
oid | proname
798+
-----+---------
799+
(0 rows)
800+
776801
-- **************** pg_opfamily ****************
777802
-- Look for illegal values in pg_opfamily fields
778803
SELECT p1.oid

‎src/test/regress/sql/aggregates.sql

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -357,14 +357,13 @@ select aggfns(distinct a,a,c order by a,b)
357357
from (values (1,1,'foo')) v(a,b,c), generate_series(1,2) i;
358358

359359
-- string_agg tests
360-
select string_agg(a)from (values('aaaa'),('bbbb'),('cccc')) g(a);
361360
select string_agg(a,',')from (values('aaaa'),('bbbb'),('cccc')) g(a);
362361
select string_agg(a,',')from (values('aaaa'),(null),('bbbb'),('cccc')) g(a);
363-
select string_agg(a,',')from (values(null),(null),('bbbb'),('cccc')) g(a);
362+
select string_agg(a,'AB')from (values(null),(null),('bbbb'),('cccc')) g(a);
364363
select string_agg(a,',')from (values(null),(null)) g(a);
365364

366365
-- check some implicit casting cases, as per bug #5564
367-
select string_agg(distinct f1order by f1)from varchar_tbl;-- ok
368-
select string_agg(distinct f1::textorder by f1)from varchar_tbl;-- not ok
369-
select string_agg(distinct f1order by f1::text)from varchar_tbl;-- not ok
370-
select string_agg(distinct f1::textorder by f1::text)from varchar_tbl;-- ok
366+
select string_agg(distinct f1,','order by f1)from varchar_tbl;-- ok
367+
select string_agg(distinct f1::text,','order by f1)from varchar_tbl;-- not ok
368+
select string_agg(distinct f1,','order by f1::text)from varchar_tbl;-- not ok
369+
select string_agg(distinct f1::text,','order by f1::text)from varchar_tbl;-- ok

‎src/test/regress/sql/opr_sanity.sql

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -621,6 +621,26 @@ WHERE a.aggfnoid = p.oid AND a.aggsortop = o.oid AND
621621
amopmethod= (SELECToidFROM pg_amWHERE amname='btree')
622622
ORDER BY1,2;
623623

624+
-- Check that there are not aggregates with the same name and different
625+
-- numbers of arguments. While not technically wrong, we have a project policy
626+
-- to avoid this because it opens the door for confusion in connection with
627+
-- ORDER BY: novices frequently put the ORDER BY in the wrong place.
628+
-- See the fate of the single-argument form of string_agg() for history.
629+
-- The only aggregates that should show up here are count(x) and count(*).
630+
631+
SELECTp1.oid::regprocedure,p2.oid::regprocedure
632+
FROM pg_procAS p1, pg_procAS p2
633+
WHEREp1.oid<p2.oidANDp1.proname=p2.pronameAND
634+
p1.proisaggANDp2.proisaggAND
635+
array_dims(p1.proargtypes)!= array_dims(p2.proargtypes)
636+
ORDER BY1;
637+
638+
-- For the same reason, aggregates with default arguments are no good.
639+
640+
SELECToid, proname
641+
FROM pg_procAS p
642+
WHERE proisaggAND proargdefaultsIS NOT NULL;
643+
624644
-- **************** pg_opfamily ****************
625645

626646
-- Look for illegal values in pg_opfamily fields

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp