Re: type cache cleanup improvements

Hi, Noah!

On Sat, Apr 12, 2025 at 12:43 AM Alexander Korotkov
<aekorotkov(at)gmail(dot)com> wrote:
>
> On Fri, Apr 11, 2025 at 11:32 PM Noah Misch <noah(at)leadboat(dot)com> wrote:
> >
> > On Tue, Oct 22, 2024 at 08:33:24PM +0300, Alexander Korotkov wrote:
> > > On Tue, Oct 22, 2024 at 6:10 PM Pavel Borisov <pashkin(dot)elfe(at)gmail(dot)com> wrote:
> > > > On Tue, 22 Oct 2024 at 11:34, Alexander Korotkov <aekorotkov(at)gmail(dot)com> wrote:
> > > >> I'm going to push this if no objections.
> >
> > (This became commit b85a9d0.)
> >
> > > + /* Call delete_rel_type_cache() if we actually cleared something */
> > > + if (hadTupDescOrOpclass)
> > > + delete_rel_type_cache_if_needed(typentry);
> >
> > I think the intent was to maintain the invariant that a RelIdToTypeIdCacheHash
> > entry exists if and only if certain kinds of data appear in the TypeCacheHash
> > entry. However, TypeCacheOpcCallback() clears TCFLAGS_OPERATOR_FLAGS without
> > maintaining RelIdToTypeIdCacheHash. Is it right to do that?
>
> Thank you for the question. I'll recheck this in next couple of days.

Sorry for the delay. Generally, your finding is correct. But, I
didn't manage to reproduce the situation, where existing code leads to
real error. In order to have it, we must have typcache entry without
TCFLAGS_HAVE_PG_TYPE_DATA and tupDesc, but with some of
TCFLAGS_OPERATOR_FLAGS. Reseting TCFLAGS_HAVE_PG_TYPE_DATA for a
composite type doesn't seem to be possible without resetting the rest
at the same time.

Nevertheless, I think it would be fragile to leave the current code
"as is". If even there is no case of real error (or it's just me
didn't manage to find it), it could appear after further changes of
type cache code. So, the fix is attached.

------
Regards,
Alexander Korotkov
Supabase