2010/1/28 David E. Wheeler <da...@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. 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@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers