Re: Consistently use the XLogRecPtrIsInvalid() macro

Hi,

On Thu, Oct 30, 2025 at 02:55:51PM +0100, Peter Eisentraut wrote:
> On 30.10.25 10:17, Bertrand Drouvot wrote:
> > - 0002 deprecates XLogRecPtrIsInvalid(): it emits a warning message at compilation
> > time if XLogRecPtrIsInvalid() is in use in the code base.
>
> Surely this could be factored out in macro in such a way that the warning
> message is a macro argument and we could reuse this attribute elsewhere in
> the code.

Yeah, I thought about it and initially considered waiting until we have another
use case. But you're right that it's better to do it from the start.

> That said, I'm suspicious about marking things deprecated before the
> replacement is widely available. If an extension has been using
> XLogRecPtrIsInvalid() until now (which has been best practice), when that
> extension adds PG19 support, they will either have to backport
> XLogRecPtrIsValid or turn off deprecation warnings.

That's a good point, thanks! I agree with your concern.

After giving it more thought, I'm inclined to postpone the compiler warning
until XLogRecPtrIsValid() has been available for some time. The question is for
how long?

I did some research, reading [1] (but that's not really identical) or looking
at what we did for example for "SPI_prepare_cursor", "SPI_prepare_params"
and friends (but again that's not really identical): 844fe9f159a mentioned
that they "can perhaps go away at some point" and are documented as
deprecated since PG14.

So, back at our case, a conservative approach would be to wait for 5
years until the new macro is available on all the supported major versions.
That would mean introduce the deprecated attribute in PG24.

I don't see waiting for PG24 as an issue: the main goal of the new macro is
to be consistent with other *IsValid() ones and avoid "double negation" and that
would be achieved in the core code base.

For PG19, we could:

Add a comment in the code documenting that XLogRecPtrIsInvalid() is deprecated
and that we will enforce a "deprecated" attribute on it in PG24.

> At least there should
> be some guidance about what you expect third-party code to do about this.

The alternative of adding the warning immediately with migration guidance would
still create "friction". Especially if we deprecate multiple APIs following this
new pattern (i.e adding a "deprecated" attribute). As you said, they could
backport the macro themselves but that seems like unnecessary friction when we
can simply wait.

What do you think?

[1]: https://www.postgresql.org/message-id/4992828D.8000307%40gmx.net

Regards,

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