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'.
> * 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.
> * 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.
Patch looks great, thank you!
In response to
pgsql-hackers by date
|Next:||From: Greg Smith||Date: 2010-01-28 04:13:05|
|Subject: Re: CommitFest status summary 2010-01-27|
|Previous:||From: Takahiro Itagaki||Date: 2010-01-28 02:47:07|
|Subject: Re: Review: listagg aggregate|