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

On 2023-06-28 We 16:54, Andres Freund wrote:
> Hi,
>
> On 2023-06-28 15:52:28 -0400, Tom Lane wrote:
>> Andres Freund<andres(at)anarazel(dot)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