Re: Consistently use the XLogRecPtrIsInvalid() macro

Hi,

On Wed, Oct 29, 2025 at 05:50:13PM +0100, Peter Eisentraut wrote:
> On 28.10.25 13:33, Bertrand Drouvot wrote:
> > I do prefer to introduce XLogRecPtrIsValid(x) and switch to that. Then, do the
> > same kind of work on OidIsValid() and TransactionIdIsValid() and add an annual
> > check.
> >
> > Idea is to get some code consistency while keeping macros which are valuable for
> > readability and centralize changes if any need to be done in the way we check
> > their validity.
>
> If we wanted real type safety, we could turn XLogRecPtr back into a struct,
> and then enforce the use of XLogRecPtrIsValid() and similar.

Right. That said I think that we'd need an opaque struct to avoid developers
doing things like: lsn.value == InvalidXLogRecPtr. If not, we'd still
need regular checks to ensure the macro is used. Opaque struct would probably
add extra costs too.

> Otherwise, we
> should just acknowledge that it's an integer and use integer code to deal
> with it. These *IsValid() and similar macros that are there for
> "readability" but are not actually enforced other than by some developers'
> willpower are just causing more work and inconsistency in the long run.

That's a good point. Scripts (like the ones shared in [1]) can catch violations,
but it's still "manual" enforcement.

We don't currently enforce the other *IsValid() macros. I think it would be worth
setting up checks for all of them, but I agree that's new ongoing maintenance
work.

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com