Re: Inval reliability, especially for inplace updates

On Wed, Jan 21, 2026 at 9:47 AM Noah Misch <noah(at)leadboat(dot)com> wrote:

> On Wed, Jan 21, 2026 at 08:59:51AM -0800, Mark Dilger wrote:
> > On Wed, Oct 23, 2024 at 7:54 PM Noah Misch <noah(at)leadboat(dot)com> wrote:
> > > I'm attaching the branch-specific patches for that and for the main
> fix.
> > > Other notes from back-patching:
> > >
> > > - All branches change the ABI of PrepareToInvalidateCacheTuple(), a
> > > function
> > > catcache.c exports for the benefit of inval.c. No PGXN extension
> calls
> > > that, and I can't think of a use case in extensions.
> >
> > Unfortunately, I can think of four.
>
> Those are non-PGXN extensions, right?
>

Right.

Based on your experience, I probably should encourage packagers to do an
> early
> check of the packages they build, especially if they build tableam modules
> not
> found in PGXN. How do you see it?
>

I don't know what you mean by "early". 18.2 hasn't stamped yet. 18.1
doesn't have the change. So, I'd say that I'm building pretty early, and I
noticed the change will be coming in 18.2.

> > I have four Table Access Methods that
> > I now need to fork to be compatible with 18.0 and 18.1 on the one hand,
> and
> > 18.2 onward on the other.
>
> For what it's worth, the ABI break you quoted is the v14-v17 break, not the
> v18 break:
>
> - v18.2 (06b030e) is changing the CacheInvalidateHeapTupleInplace() ABI
> - v14-v17 (e.g. 2e58802) is changing the PrepareToInvalidateCacheTuple()
> ABI
>

I'll have to work around both. I maintain TAM packages going back multiple
major versions.

> > I'm sorry I didn't follow this thread before it got pushed.
> >
> > Is there a reason for doing this change in back branches? The thread is
> > pretty long, and I'm struggling to find a security or stability
> > justification for the ABI break, but perhaps there is one.
>
> Chiefly, the fix prevents data loss that arose via losing a relhasindex,
> relfrozenxid, or datfrozenxid update. (The log message of 0f69bed says
> "another backend's DDL could then update the row without incorporating the
> inplace update".) For an example, see where that commit edits
> src/test/isolation/expected/inplace-inval.out.
>

Oh, I don't mean to question the overall purpose of the patch. I was
questioning whether it needed to have breaking changes which are mere "code
cleanup". The change to CacheInvalidateHeapTupleInplace to remove the
unused third argument seemed inappropriate for backpatching, so I spoke up
before 18.2 is stamped. Doing this one piece of code cleanup in the back
branches will cause a lot of packaging pain for no real benefit.

--

*Mark Dilger*