Review: listagg aggregate
in [PgSql]
|
From: Pavel Stehule on 28 Jan 2010 03:40 2010/1/28 Pavel Stehule <pavel.stehule(a)gmail.com>: > 2010/1/28 David E. Wheeler <david(a)kineticode.com>: >> On Jan 27, 2010, at 6:47 PM, Takahiro Itagaki wrote: >> >>> * I think we cannot cache the delimiter at the first call. >>> Â For example, >>> Â Â SELECT string_agg(elem, delim) >>> Â Â Â FROM (VALUES('A', ','), ('B', '+'), ('C', '*')) t(elem, delim); >>> Â should return 'A+B*C' rather than 'A,B,C'. >> >> Ooh, nice. >> >>> * Can we use StringInfo directly as the aggregate context instead of >>> Â StringAggState? For the first reason, we need to drop 'delimiter' field >>> Â from struct StringAggState. Now it has only StringInfo field. >> >> Makes sense. > > no, has not. What is use case for this behave?? Pavel > > Pavel > >> >>> * We'd better avoiding to call text_to_cstring() for delimitors and elements >>> Â for performance reason. We can use appendBinaryStringInfo() here. >>> >>> My proposal patch attached. >>> >>> Also, I've not changed it yet, but it might be considerable: >>> >>> * Do we need better names for string_agg1_transfn and string_agg2_transfn? >>> Â They are almost "internal names", but we could have more >>> Â like string_agg_with_sep_transfn. >> >> Yes please. >> >>> Comments? >> >> Patch looks great, thank you! >> >> 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: Takahiro Itagaki on 28 Jan 2010 03:59 Pavel Stehule <pavel.stehule(a)gmail.com> wrote: > 2010/1/28 Pavel Stehule <pavel.stehule(a)gmail.com>: > > 2010/1/28 David E. Wheeler <david(a)kineticode.com>: > >> On Jan 27, 2010, at 6:47 PM, Takahiro Itagaki wrote: > >> > >>> * I think we cannot cache the delimiter at the first call. > >>> For example, > >>> SELECT string_agg(elem, delim) > >>> FROM (VALUES('A', ','), ('B', '+'), ('C', '*')) t(elem, delim); > >>> should return 'A+B*C' rather than 'A,B,C'. > > > > no, has not. > What is use case for this behave?? I also think this usage is nonsense, but seems to be the most consistent behavior for me. I didn't say anything about use-cases, but just capability. Since we allow such kinds of usage for now, you need to verify the delimiter is not changed rather than ignoring it if you want disallow to change the delimiter during an aggregation. Of course you can cache the first delimiter at start, and check delimiters are not changed every calls -- but I think it is just a waste of cpu cycle. Regards, --- Takahiro Itagaki NTT Open Source Software Center -- 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: Robert Haas on 28 Jan 2010 08:53 On Thu, Jan 28, 2010 at 3:59 AM, Takahiro Itagaki <itagaki.takahiro(a)oss.ntt.co.jp> wrote: > Pavel Stehule <pavel.stehule(a)gmail.com> wrote: > >> 2010/1/28 Pavel Stehule <pavel.stehule(a)gmail.com>: >> > 2010/1/28 David E. Wheeler <david(a)kineticode.com>: >> >> On Jan 27, 2010, at 6:47 PM, Takahiro Itagaki wrote: >> >> >> >>> * I think we cannot cache the delimiter at the first call. >> >>> For example, >> >>> SELECT string_agg(elem, delim) >> >>> FROM (VALUES('A', ','), ('B', '+'), ('C', '*')) t(elem, delim); >> >>> should return 'A+B*C' rather than 'A,B,C'. >> > >> > no, has not. >> What is use case for this behave?? > > I also think this usage is nonsense, but seems to be the most consistent > behavior for me. I didn't say anything about use-cases, but just capability. > Since we allow such kinds of usage for now, you need to verify the > delimiter is not changed rather than ignoring it if you want disallow > to change the delimiter during an aggregation. > > Of course you can cache the first delimiter at start, and check delimiters > are not changed every calls -- but I think it is just a waste of cpu cycle. Agreed. Not caching it seems the simplest solution. ....Robert -- 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 28 Jan 2010 09:01 2010/1/28 Robert Haas <robertmhaas(a)gmail.com>: > On Thu, Jan 28, 2010 at 3:59 AM, Takahiro Itagaki > <itagaki.takahiro(a)oss.ntt.co.jp> wrote: >> Pavel Stehule <pavel.stehule(a)gmail.com> wrote: >> >>> 2010/1/28 Pavel Stehule <pavel.stehule(a)gmail.com>: >>> > 2010/1/28 David E. Wheeler <david(a)kineticode.com>: >>> >> On Jan 27, 2010, at 6:47 PM, Takahiro Itagaki wrote: >>> >> >>> >>> * I think we cannot cache the delimiter at the first call. >>> >>> Â For example, >>> >>> Â Â SELECT string_agg(elem, delim) >>> >>> Â Â Â FROM (VALUES('A', ','), ('B', '+'), ('C', '*')) t(elem, delim); >>> >>> Â should return 'A+B*C' rather than 'A,B,C'. >>> > >>> > no, has not. >>> What is use case for this behave?? >> >> I also think this usage is nonsense, but seems to be the most consistent >> behavior for me. I didn't say anything about use-cases, but just capability. >> Since we allow such kinds of usage for now, you need to verify the >> delimiter is not changed rather than ignoring it if you want disallow >> to change the delimiter during an aggregation. >> >> Of course you can cache the first delimiter at start, and check delimiters >> are not changed every calls -- but I think it is just a waste of cpu cycle. > > Agreed. Â Not caching it seems the simplest solution. simplest could not be a best. There have to be only a const expression. But we have not possibility to check it in pg. Pavel > > ...Robert > -- 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: Robert Haas on 28 Jan 2010 09:17
On Thu, Jan 28, 2010 at 9:01 AM, Pavel Stehule <pavel.stehule(a)gmail.com> wrote: > simplest could not be a best. There have to be only a const > expression. But we have not possibility to check it in pg. Well... that's an entirely arbitrary limitation. I admit that it doesn't seem likely that someone would want to have a variable delimiter, but putting extra effort and code complexity into preventing it seems pointless. ....Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers |