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(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?

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