Re: lazy detoasting

Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com> writes:
> Here is a more complete patch. I made a call graph to get to the
> bottom, literally, of how variable assignments happen in PL/pgSQL. (See
> attached.) There are four leaf functions to patch up.
> Also, I wrote some isolation tests to hit each of these cases. I wasn't
> able to construct one for expanded_record_set_fields(), but the
> principle there should be the same.

I don't like this patch too much: it leaks memory in some places where
that's not a good idea, and in expanded_record_set_field_internal,
it's actually allocating the new field value in the wrong context.
And it pays no attention to updating comments, even ones that it's
flat out invalidated.

I also realized while poking at it that it'd broken much of the work
we did to optimize array and record variables in plpgsql by storing
them in "expanded" form, because in a non-atomic context it causes
assign_simple_var to forcibly flatten anything that heap_tuple_fetch_attr
knows how to flatten --- including "expanded" datums.

To make that last case work properly, we want assign_simple_var to only
flatten things that are actually ONDISK or INDIRECT external datums.
It's okay to leave "expanded" datums as-is, because they're just in memory
and as long as we haven't screwed up memory management they'll be fine
across a transaction boundary.

I made a new macro in postgres.h to recognize such cases, but I wonder
if anyone thinks we should do that differently. It seems a bit odd
that we've not needed to make that distinction before. OTOH, flattening
all external datums does seem like the right semantics for the
expandedrecord.c functions to have, so maybe it's fine.

Anyway, attached is a revised patch. I found a test case for
expanded_record_set_fields(), too.

regards, tom lane