Re: type cache cleanup improvements

On 25/8/2024 23:22, Alexander Korotkov wrote:
> On Sun, Aug 25, 2024 at 10:21 PM Alexander Korotkov
>>> (This Assert is introduced by c14d4acb8.)
>>
>> Thank you for noticing.  I'm checking this.
> 
> I didn't take into account that TypeCacheEntry could be invalidated
> while lookup_type_cache() does syscache lookups.  When I realized that
> I was curious on how does it currently work.  It appears that type
> cache invalidation mostly only clears the flags while values are
> remaining in place and still available for lookup_type_cache() caller.
> TypeCacheEntry.tupDesc is invalidated directly, and it has guarantee
> to survive only because we don't do any syscache lookups for composite
> data types later in lookup_type_cache().  I'm becoming less fan of how
> this works...  I think these aspects needs to be at least documented
> in details.
> 
> Regarding c14d4acb8, it appears to require redesign.  I'm going to revert it.
Sorry, but I don't understand your point.
Let's refocus on the problem at hand. The issue arose when the 
TypeCacheTypCallback and the TypeCacheRelCallback were executed in 
sequence within InvalidateSystemCachesExtended.
The first callback cleaned the flags TCFLAGS_HAVE_PG_TYPE_DATA and 
TCFLAGS_CHECKED_DOMAIN_CONSTRAINTS. But the call of the second callback 
checks the typentry->tupDesc and, because it wasn't NULL, attempted to 
remove this record a second time.
I think there is no case for redesign, but we have a mess in 
insertion/deletion conditions.

-- 
regards, Andrei Lepikhov