On 26 March 2018 at 15:26, Stephen Frost <sfr...@snowman.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
