Re: type cache cleanup improvements

Hi, Noah!

On Tue, Apr 29, 2025 at 3:56 AM Noah Misch <noah@leadboat.com> wrote:
> 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.

Thank you for your feedback!

------
Regards,
Alexander Korotkov
Supabase