On 2 March 2018 at 10:26, Andres Freund <andres@anarazel.de> wrote:
> On 2017-12-18 03:30:55 +1300, David Rowley wrote:
>> Just a handful of aggregates now don't support partial aggregation;
>>
>> postgres=# select aggfnoid from pg_aggregate where aggcombinefn=0 and
>> aggkind='n';
>> aggfnoid
>> ------------------
>> xmlagg
>> json_agg
>> json_object_agg
>> jsonb_agg
>> jsonb_object_agg
>> (5 rows)
>
> FWIW, I've heard numerous people yearn for json*agg
I guess it'll need to be PG12 for now. I'd imagine string_agg and
array_agg are more important ones to tick off for now, but I bet many
people would disagree with that.
>
>> I'm going to add this to PG11's final commitfest rather than the
>> January 'fest as it seems more like a final commitfest type of patch.
>
> Not sure I understand?
hmm, yeah. That was more of a consideration that I should be working
hard on + reviewing more complex and invasive patches before the final
'fest. I saw this more as something that would gain more interest once
patches such as partition-wise aggs gets in.
>> + /* XXX is there anywhere to cache this to save calling each time? */
>> + getTypeBinaryOutputInfo(state->element_type, &typsend, &typisvarlena);
>
> Yes, you should be able to store this in fcinfo->flinfo->fn_extra. Or do
> you see a problem doing so?
No, just didn't think of it.
>> + /* XXX? Surely this cannot be the way to do this? */
>> + StringInfoData tmp;
>> + tmp.cursor = 0;
>> + tmp.maxlen = tmp.len = pq_getmsgint(&buf, 4);
>> + tmp.data = (char *) pq_getmsgbytes(&buf, tmp.len);
>> +
>> + result->dvalues[i] = OidReceiveFunctionCall(typreceive, &tmp,
>> +
typioparam,-1);
>
> Hm, doesn't look too bad to me? What's your concern here?
I was just surprised at having to fake up a StringInfoData.
> This isn't a real review, I was basically just doing a quick
> scroll-through.
Understood.
I've attached a delta patch which can be applied on top of the v2
patch which removes that XXX comment above and also implements the
fn_extra caching.
Thanks for looking!
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services