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

Hi,

On 2023-06-28 15:52:28 -0400, Tom Lane wrote:

Andres Freund <andres@anarazel.de> writes:

If other sessions caused the tupledesc to be changed,
we should already hang onto the old definition via the
RememberToFreeTupleDescAtEOX() mechanism?

I believe the tupdesc in question is actually in the typcache,
which doesn't have anything like RememberToFreeTupleDescAtEOX
(which is a horrid hack anyway if you ask me).

It's in the typecache, but that just uses the relcache's tupledesc for
non-record composites. But looks like it doesn't suffice, because
TypeCacheRelCallback() releases the refcount the typecache held, regardless of
the tupledesc having changed meaningfully or not.

So even if there can't have been "important" changes to the tupledesc due to
locking, we end up with the newer tupledesc on a second lookup...


I agree that the RememberToFreeTupleDescAtEOX thing is a ugly hack, but I
don't think it's easy to come up with something good...

We could probably make things better for this specific case by
teaching the typcache not to replace a cached tupdesc unless its
contents actually change.  But that just makes it harder to get
to a bug instance; it's not a cure-all.

Yea :(.

:-(

I thought about  whether the datumCopy() idea might be manageable, but getmissingattr() is inlined by heap_getttr() which has distressingly large number of call sites, including in third party code.

Could we maybe cache missing values elsewhere in a way that's less volatile? That's an extremely vague idea, just thinking out loud.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com