Re: In core use of RegisterXactCallback() and RegisterSubXactCallback()

On Fri, Jun 26, 2026 at 12:05 AM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:
> On Thu, Jun 25, 2026 at 10:17:13PM +0900, Amit Langote wrote:
> > On Thu, Jun 25, 2026 at 2:25 PM Amit Langote <amitlangote09@gmail.com> wrote:
> > >
> > > Note the RegisterSubXactCallback() call from b7b27eb41a5 was already
> > > removed by a later fix (4113873a) that confined fast-path batching to
> > > the top transaction level, so only the RegisterXactCallback() remains.
> > > You're right that the header comment points toward hardwiring for
> > > built-in code. I took the shortcut of using Register* because the RI
> > > fast-path callback has no ordering dependency on the other hard-wired
> > > work in Commit/AbortTransaction(), but I agree it should follow the
> > > convention. I'll work up a patch converting it to a hardwired
> > > AtEOXact_RI() called from CommitTransaction(), PrepareTransaction()
> > > and AbortTransaction(), and post it on this thread.
> >
> > Here is a patch to do so.
>
> Thanks for looking at it!
>
> > It's a mechanical conversion, except the new AtEOXact_RI() takes
> > isCommit and asserts that the fast-path cache was already torn down by
> > commit, with a WARNING in non-assert builds.
>
> That makes sense to me.

Thanks for checking.

> Just one nit:
>
> + Assert for development builds and, since
> +        * the transaction is already committed by now and FK checks may have been
> +        * skipped, also warn in production builds.
>
> s/development builds/assert-enabled builds/? That looks more common and "
> development builds" would be introduced by this patch.

I grepped, and while the word "development" isn't rare in the tree,
none of the uses are "development builds" as a build type.
"assert-enabled" is the established term for that, so I've switched to
it in v2.


--
Thanks, Amit Langote