On 2023-06-28 We 17:57, Andrew Dunstan wrote:
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@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.
After (not) sleeping on this overnight, and discussing it with a colleague this morning, here's a suggestion. We have a hash table, keyed by (tdtypeid, attnum) where we store a datumCopy'd version of the value. If it's present just return the value instead of getting it from the tupdesc. The hash table is blown away at the end of the transaction. Assuming that's workable I think it would not be a large patch.
My colleague did ask if we had live reproducible case.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com