Re: type cache cleanup improvements

On Mon, Apr 21, 2025 at 04:54:08AM +0300, Alexander Korotkov wrote:
> On Sat, Apr 12, 2025 at 12:43 AM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> > On Fri, Apr 11, 2025 at 11:32 PM Noah Misch <noah@leadboat.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.elfe@gmail.com> wrote:
> > > > > On Tue, 22 Oct 2024 at 11:34, Alexander Korotkov <aekorotkov@gmail.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?

> 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.

That makes sense.

> 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.

This change looks appropriate.  Thanks.