On Sun, May 7, 2017 at 3:48 PM, Tom Lane <t...@sss.pgh.pa.us> wrote:
> Mat Arye <m...@timescale.com> writes:
> > This is in arrayfuncs.c:5022 (postgre 9.6.2)
> > /*
> > * Ensure pass-by-ref stuff is copied into mcontext; and detoast it too if
> > * it's varlena. (You might think that detoasting is not needed here
> > * because construct_md_array can detoast the array elements later.
> > * However, we must not let construct_md_array modify the ArrayBuildState
> > * because that would mean array_agg_finalfn damages its input, which is
> > * verboten. Also, this way frequently saves one copying step.)
> > */
> > I am a bit confused by the comment.
> > Does PG_DETOAST_DATUM_COPY(dvalue) modify dvalue?
> What the comment is on about is that construct_md_array does this:
> /* make sure data is not toasted */
> if (elmlen == -1)
> elems[i] = PointerGetDatum(PG_DETOAST_DATUM(elems[i]));
> that is, it intentionally modifies the passed-in elems array in
> the case that one of the elements is a toasted value. For most
> callers, the elems array is only transient storage anyway. But
> it's problematic for makeMdArrayResult, because it would mean
> that the ArrayBuildState is changed --- and while it's not changed
> in a semantically significant way, that's still a problem, because
> the detoasted value would be allocated in a context that is possibly
> shorter-lived than the ArrayBuildState is. (In a hashed aggregation
> situation, the ArrayBuildState is going to live in storage that is
> persistent across the aggregation, but we are calling makeMdArrayResult
> in the context where we want the result value to be created, which
> is of only per-output-tuple lifespan.)
> You could imagine fixing this by having construct_md_array do some
> context switches internally, but that would complicate it. And in
> any case, fixing the problem where it is allows us to combine the
> possible detoasting with copying the value into the ArrayBuildState's
> context, so we'd probably keep doing that even if it was safe not to.
Thanks. That clears it up.
> > The thing I am struggling with is with the serialize/deserialize
> > Can I detoast inside the serialize function if I use _COPY? Or is there a
> > reason I have to detoast during the sfunc?
> Should be able to detoast in serialize if you want. Are you having
> trouble with that? (It might be inefficient to do it that way, if
> you have to serialize the same value multiple times. But I'm not
> sure if that can happen.)
I haven't run into any actual problems yet, just wanted to figure out a
clean mental model before implementing. Thanks a lot for the clarification.
> regards, tom lane