On 2 March 2018 at 10:26, Andres Freund <and...@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
combinefn_for_string_and_array_aggs_v3_delta.patch
Description: Binary data
