On Wed, Jan 28, 2026 at 12:54 AM Andres Freund <andres@anarazel.de> wrote:
>
> Hi,
>
> On 2026-01-27 16:40:55 +0530, Dilip Kumar wrote:
> > > > I know why the check exists - but why does it have to be in
> > > > table_scan_getnextslot(), which is executed very frequently, rather than
> > > > table_beginscan*(), which is executed much less frequently.
> > > >
> > >
> > > I thought about this point and couldn't think of any reason why this
> > > check can't be in table_beginscan*(). I think your idea of having a
> > > wrapper around scan_begin() to handle this check is a good one.
> >
> > Here is the patch. I've used table_scan_begin_wrapper() to wrap the
> > scan_begin() callback for now. If you have a better naming preference
> > to avoid the 'wrapper' suffix, please let me know.
>
> I'd probably just go for _internal(), _impl() or such.
Thanks
>
> > diff --git a/src/backend/access/table/tableam.c b/src/backend/access/table/tableam.c
> > index 87491796523..15a92c052d3 100644
> > --- a/src/backend/access/table/tableam.c
> > +++ b/src/backend/access/table/tableam.c
> > [...]
> > +/*
> > + * table_scan_begin_wrapper
> > + *
> > + * A wrapper around the Table Access Method scan_begin callback. This handles
> > + * centralized error checking—specifically ensuring we aren't performing
> > + * table scan while CheckXidAlive is valid. This state is reserved for
> > + * specific logical decoding operations where direct relation scanning is
> > + * prohibited.
> > + */
> > +TableScanDesc
> > +table_scan_begin_wrapper(Relation rel, Snapshot snapshot, int nkeys,
> > + ScanKeyData *key, ParallelTableScanDesc pscan,
> > + uint32 flags)
> > +{
> > + /*
> > + * We don't expect direct calls to this function with valid CheckXidAlive
> > + * for catalog or regular tables. See detailed comments in xact.c where
> > + * these variables are declared.
> > + */
> > + if (unlikely(TransactionIdIsValid(CheckXidAlive) && !bsysscan))
> > + elog(ERROR, "unexpected table_scan_begin_wrapper call during logical decoding");
> > +
> > + return rel->rd_tableam->scan_begin(rel, snapshot, nkeys, key, pscan, flags);
> > +}
>
> Given that some of the callers are in inline functions in tableam.h, I would
> implement the wrapper there. I doubt it'll make a meaningful difference, but
> it doesn't seem worth "risking" that.
Yeah, it makes sense, changed.
--
Regards,
Dilip Kumar
Google
