Review: listagg aggregate
in [PgSql]
|
From: Takahiro Itagaki on 31 Jan 2010 22:29 Robert Haas <robertmhaas(a)gmail.com> wrote: > > ok - I am only one who like original behave - so I ?am withdrawing. > > Robert, If you like, please commit Itagaki's patch. + add note about > > behave when second parameter isn't stable. > > I haven't even looked at this code - I sort of assumed Itagaki was > handling this one. But it might be good to make sure that the docs > have been read through by a native English speaker prior to commit... Applied with some editorialization. Please check the docs are good enough. I removed a test pattern for variable delimiters from regression test because it might be an undefined behavior. We might change the behavior in the future, so we guarantee only constant delimiters. The transition functions are renamed to "string_agg_transfn" and "string_agg_delim_transfn". We cannot use "string_agg_transfn" for both names because we cast the function name as regproc in bootstrap; it complains about duplicated function names. 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: Pavel Stehule on 1 Feb 2010 00:07 2010/2/1 Takahiro Itagaki <itagaki.takahiro(a)oss.ntt.co.jp>: > > Robert Haas <robertmhaas(a)gmail.com> wrote: > >> > ok - I am only one who like original behave - so I ?am withdrawing. >> > Robert, If you like, please commit Itagaki's patch. + add note about >> > behave when second parameter isn't stable. >> >> I haven't even looked at this code - I sort of assumed Itagaki was >> handling this one. Â But it might be good to make sure that the docs >> have been read through by a native English speaker prior to commit... > > Applied with some editorialization. Please check the docs are good enough. > > I removed a test pattern for variable delimiters from regression test > because it might be an undefined behavior. We might change the behavior > in the future, so we guarantee only constant delimiters. > > The transition functions are renamed to "string_agg_transfn" and > "string_agg_delim_transfn". We cannot use "string_agg_transfn" for > both names because we cast the function name as regproc in bootstrap; > it complains about duplicated function names. thank you very much Pavel Stehule > > 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: Thom Brown on 1 Feb 2010 07:11 I noticed that the regression test results don't include the following case: select string_agg(a) from (values(null),(null)) g(a); There is one similar where a delimiter is provided. Which leads me to ask for clarification, would it be safe to assume that string_agg can never itself return null? Thom
From: Pavel Stehule on 1 Feb 2010 08:40 2010/2/1 Thom Brown <thombrown(a)gmail.com>: > I noticed that the regression test results don't include the following case: > > select string_agg(a) from (values(null),(null)) g(a); > > There is one similar where a delimiter is provided. > > Which leads me to ask for clarification, would it be safe to assume that > string_agg can never itself return null? if all values are null, then result is null. Pavel > > Thom > > -- 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: Thom Brown on 1 Feb 2010 08:51
On 1 February 2010 13:40, Pavel Stehule <pavel.stehule(a)gmail.com> wrote: > 2010/2/1 Thom Brown <thombrown(a)gmail.com>: > > I noticed that the regression test results don't include the following > case: > > > > select string_agg(a) from (values(null),(null)) g(a); > > > > There is one similar where a delimiter is provided. > > > > Which leads me to ask for clarification, would it be safe to assume that > > string_agg can never itself return null? > > if all values are null, then result is null. > > Pavel > > > Ah, I was looking at the expected results, and couldn't see a NULL outcome, but then these aren't indicated in such results anyway. Thom |