Re: BUG #17994: Invalidating relcache corrupts tupDesc inside ExecEvalFieldStoreDeForm()

PG Bug reporting form <noreply(at)postgresql(dot)org> writes:
> The following script:
> ...
> invokes errors or a server crash:

Thanks for the report! The good news is that the case shown here
is easily fixed, as attached. get_cached_rowtype() explicitly
specifies that its result doesn't hold good across database
access if you don't increment the tupdesc's refcount. Most of
the callers get this right, but ExecEvalFieldStoreDeForm() is
doing things in the wrong order by fetching the tupdesc before
detoasting the input datum. We could fix that by temporarily
incrementing the refcount, but I don't see any reason we can't
just reorder the steps to make it safe.

The bad news is that while investigating this I came across
another hazard that seems much messier to fix. To wit, if
you have a tuple with "missing" pass-by-ref columns, then
some of the columns output by heap_deform_tuple() will
contain pointers into the tupdesc (cf. getmissingattr()).
That's not sufficient lifespan in the scenario we're dealing
with here: if the tupdesc gets trashed anytime before the
eventual ExecEvalFieldStoreForm(), we'll have garbage in the
result tuple.

Worse, I fear there may be many other places with latent bugs of the
same ilk. Before the attmissingval code was added, one could assume
that the result of heap_deform_tuple would hold good as long as you
had a pin on the source tuple. But now it depends on metadata as
well, and I don't think we have a coherent story about that.

Any thoughts what to do? I considered making getmissingattr
apply datumCopy, but that would probably lead to unpleasant
memory leaks.

regards, tom lane