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. > * 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