Greetings, * David Rowley (david.row...@2ndquadrant.com) wrote: > On 16 March 2018 at 13:46, Tomas Vondra <tomas.von...@2ndquadrant.com> wrote: > > I've done more testing on this patch, and I haven't found any new issues > > with it, so PFA I'm marking it as ready for committer. > > Great! Many thanks for the review.
I read through the thread and started reviewing this, comments below. diff --git a/src/backend/utils/adt/array_userfuncs.c b/src/backend/utils/adt/array_userfuncs.c [...] + for (i = 0; i < state2->nelems; i++) + state1->dvalues[i] = datumCopy(state2->dvalues[i], + state1->typbyval, state1->typlen); + + memcpy(state1->dnulls, state2->dnulls, sizeof(bool) * state2->nelems); 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. [...] + for (i = 0; i < state->nelems; i++) + { + outputbytes = OidSendFunctionCall(iodata->typsend, state->dvalues[i]); + pq_sendint32(&buf, VARSIZE(outputbytes) - VARHDRSZ); + pq_sendbytes(&buf, VARDATA(outputbytes), + VARSIZE(outputbytes) - VARHDRSZ); + } + } + + /* dnulls */ + pq_sendbytes(&buf, (char *) state->dnulls, sizeof(bool) * state->nelems); SendFunctionCall() has a similar "Do not call this on NULL datums" comment. + for (i = 0; i < nelems; i++) + { + 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(iodata->typreceive, + &tmp, + iodata->typioparam, + -1); + } + } + + /* dnulls */ + temp = pq_getmsgbytes(&buf, sizeof(bool) * nelems); + memcpy(result->dnulls, temp, sizeof(bool) * nelems); And if we aren't sending them then we probably need to do something different on the receive side... I glanced through the rest and it seemed alright, but will want to still go over it more carefully once the above comments are addressed. Marking Waiting-for-Author. Thanks! Stephen
signature.asc
Description: PGP signature