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
