Review: listagg aggregate
in [PgSql]
|
From: Pavel Stehule on 25 Jan 2010 09:12 2010/1/25 Tom Lane <tgl(a)sss.pgh.pa.us>: > Pavel Stehule <pavel.stehule(a)gmail.com> writes: >> 2010/1/25 Peter Eisentraut <peter_e(a)gmx.net>: >>> xmlagg -> concatenates values to form xml datum >>> array_agg -> concatenates values to form array datum >>> ??? -> concatenates values to form string datum >>> >>> So it's pretty clear that listagg does not fit into this scheme. > >> when you define list as text domain, then this the name is correct. > > IOW, if you define away the problem then there's no problem? > > I agree that "list" is a terrible choice of name here. Â "string_agg" > seemed reasonable and in keeping with the standardized "array_agg". > I am not happy, I thing so we do bigger chaos then it is. But it hasn't progress. So I agree with name string_agg. In this case isn't a problem rename this function if somebody would. I'll send patch over hour. regards Pavel Stehule > Â Â Â Â Â Â Â Â Â Â Â Â regards, tom lane > -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Pavel Stehule on 25 Jan 2010 09:56 2010/1/25 Tom Lane <tgl(a)sss.pgh.pa.us>: > Pavel Stehule <pavel.stehule(a)gmail.com> writes: >> 2010/1/25 Peter Eisentraut <peter_e(a)gmx.net>: >>> xmlagg -> concatenates values to form xml datum >>> array_agg -> concatenates values to form array datum >>> ??? -> concatenates values to form string datum >>> >>> So it's pretty clear that listagg does not fit into this scheme. > >> when you define list as text domain, then this the name is correct. > > IOW, if you define away the problem then there's no problem? > > I agree that "list" is a terrible choice of name here. Â "string_agg" > seemed reasonable and in keeping with the standardized "array_agg". actualised patch - the name is string_agg regards Pavel Stehule > > Â Â Â Â Â Â Â Â Â Â Â Â regards, tom lane >
From: Pavel Stehule on 25 Jan 2010 10:04 2010/1/24 Tom Lane <tgl(a)sss.pgh.pa.us>: > Simon Riggs <simon(a)2ndQuadrant.com> writes: >> On Fri, 2010-01-22 at 11:14 -0800, David E.Wheeler wrote: >>> No performance issues > >> ISTM that this class of function is inherently dangerous performance >> wise. > >> * It looks incredibly easy to construct enormous lists. We should test >> the explosion limit of this to see how it is handled. Perhaps we need >> some parameter limits to control that, depending upon results. > >> * Optimizer doesn't consider whether the result type of an aggregate get >> bigger as the aggregate processes more rows. If we're adding this >> function we should give some thought in that area also, or at least a >> comment to note that it can and will cause the optimizer problems in >> complex queries. > > We have that problem already with array_agg(), and I don't recall many > complaints about it. Â It might be worth worrying about at some point, > but I don't think it's reasonable to insist that it be fixed before > any more such aggregates are created. > > I agree that testing-to-failure would be a good idea just to be sure it > fails cleanly. postgres=# \timing Timing is on. postgres=# select pg_size_pretty(length(string_agg('012345678901234567890'::text,','))) from generate_series(1,10000000) g(i); pg_size_pretty ---------------- 210 MB (1 row) Time: 5831,218 ms postgres=# select pg_size_pretty(length(string_agg('012345678901234567890'::text,','))) from generate_series(1,50000000) g(i); ^[^[ERROR: out of memory DETAIL: Cannot enlarge string buffer containing 1073741812 bytes by 21 more bytes. postgres=# I thing, so 210 MB is more then is necessary :) Regards Pavel Stehule > > Â Â Â Â Â Â Â Â Â Â Â Â regards, tom lane > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers > -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: "David E. Wheeler" on 25 Jan 2010 14:27 On Jan 25, 2010, at 2:09 AM, Peter Eisentraut wrote: > xmlagg -> concatenates values to form xml datum > array_agg -> concatenates values to form array datum > ??? -> concatenates values to form string datum concat_agg(). David -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: "David E. Wheeler" on 25 Jan 2010 14:29
On Jan 25, 2010, at 6:12 AM, Pavel Stehule wrote: > I am not happy, I thing so we do bigger chaos then it is. But it > hasn't progress. So I agree with name string_agg. In this case isn't a > problem rename this function if somebody would. Could you not use CREATE AGGREGATE to create an alias named listagg if you needed it? Or are we limited to a single argument in that case? David -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers |