Re: Parallel Aggregates for string_agg and array_agg

On 26 March 2018 at 15:26, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> The header at the top of datumCopy() pretty clearly says that it's for
> "non-NULL" datums, yet this function seems to be happily ignoring that
> and just trying to use it to copy everything. Perhaps I'm missing
> something, but I don't see anything obvious that would lead me to
> conclude that what's being done here (or in other similar cases in this
> patch) is acceptable.

Thanks for looking at this.

You're right. I've overlooked this. The code should be checking for
NULL value Datums there. I've fixed this locally, but on testing, I
discovered another bug around string_agg. At the moment string_agg's
transfn only allocates the state when it gets a non-NULL string to
aggregate, whereas it seems other trans functions which return an
internal state allocate their state on the first call. e.g.
int8_avg_accum(). This NULL state is causing the serial function
segfault on a null pointer dereference. I think the fix is to always
allocate the state in the transfn, but I just wanted to point this out
before I go and do that.

--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services