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

Andrew Dunstan <andrew@dunslane.net>

From: Andrew Dunstan <andrew@dunslane.net>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Andres Freund <andres@anarazel.de>, exclusion@gmail.com, pgsql-bugs@lists.postgresql.org
Date: 2023-06-29T18:57:35Z
Lists: pgsql-bugs
On 2023-06-29 Th 10:26, Tom Lane wrote:
> Andrew Dunstan<andrew@dunslane.net>  writes:
>> 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.
> That sounds possibly workable.


OK, good, we have a plan.


> I'm a bit concerned about added
> overhead, and about whether the hashtable needs invalidation support.
> It might be better to key it off (relfilenode, attnum).


re overhead: getmissingattr isn't called at all except for "off the end" 
attributes. Setting up the hash table at most once per txn doesn't seem 
likely to cost much, and even that won't be done if getmissingattr isn't 
called.

re relfilenode: we don't have it in getmissingattr, so that would 
involve looking it up or whacking around the API. Neither of those seem 
good. Do you have any reason for thinking tdtypeid will not be sufficient?

re invalidation: that seems to suggest that the missing value could 
change under us. I don't think it can, but if it can then more than just 
this is broken. If not, how would invalidation affect us?


cheers


andrew

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

Commits

  1. Cache by-reference missing values in a long lived context

  2. Fix order of operations in ExecEvalFieldStoreDeForm().