Re: Review: listagg aggregate - Mailing list pgsql-hackers

From Pavel Stehule
Subject Re: Review: listagg aggregate
Date
Msg-id 162867791001280040r4f531ed7v141462b85fc17a33@mail.gmail.com
Whole thread Raw
In response to Re: Review: listagg aggregate  (Pavel Stehule <pavel.stehule@gmail.com>)
Responses Re: Review: listagg aggregate  (Takahiro Itagaki <itagaki.takahiro@oss.ntt.co.jp>)
List pgsql-hackers
2010/1/28 Pavel Stehule <pavel.stehule@gmail.com>:
> 2010/1/28 David E. Wheeler <david@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
>>
>>
>>
>


pgsql-hackers by date:

Previous
From: Pavel Stehule
Date:
Subject: Re: Review: listagg aggregate
Next
From: Guillaume Lelarge
Date:
Subject: Re: Patch: libpq new connect function (PQconnectParams)