From: Heikki Linnakangas <hlinnaka@iki.fi>
To: Quan Zongliang <quanzongliang@yeah.net>
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: pgsql-hackers@lists.postgresql.org
Subject: Re: Consistently use the XLogRecPtrIsInvalid() macro
Date: Tue, 28 Oct 2025 13:40:24 +0200
Message-ID: <b2ceeea0-abc5-49bb-ac0a-acd84940fc5c@iki.fi> (raw)
In-Reply-To: <20213178-92cd-4ab9-9699-c68cd75bae58@yeah.net>
References: <aQB7EvGqrbZXrMlg@ip-10-97-1-34.eu-west-3.compute.internal>
<20213178-92cd-4ab9-9699-c68cd75bae58@yeah.net>
On 28/10/2025 10:53, Quan Zongliang wrote:
> On 10/28/25 4:13 PM, Bertrand Drouvot wrote:
>> While working on refactoring some code in [1], one of the changes was:
>>
>> - if (initial_restart_lsn != InvalidXLogRecPtr &&
>> - initial_restart_lsn < oldestLSN)
>> + XLogRecPtr restart_lsn = s->data.restart_lsn;
>> +
>> + if (restart_lsn != InvalidXLogRecPtr &&
>> + restart_lsn < oldestLSN)
>>
>> Sawada-san suggested to use the XLogRecPtrIsInvalid() macro here.
>>
>> But this != InvalidXLogRecPtr check was existing code, so why not
>> consistently use XLogRecPtrIsInvalid() where we check equality
>> against InvalidXLogRecPtr?
>>
>> At the time the current XLogRecPtrIsInvalid() has been introduced
>> (0ab9d1c4b316) all the InvalidXLogRecPtr equality checks were done
>> using XLogRecPtrIsInvalid().
>>
>> But since, it has changed: I looked at it and this is not the case
>> anymore in 20 files.
>>
>> PFA, patches to $SUBJECT. To ease the review, I created one patch
>> per modified file.
>>
>> I suspect the same approach could be applied to some other macros
>> too. Let's start with XLogRecPtrIsInvalid() first.
>
> Agree. This patch has made the code style more consistent. And using
> such macros is also in line with the usual practice. Just like
> OidIsValid and TransactionIdIsValid.
Back in the day, XLogRecPtrIsInvalid() was needed because XLogRecPtr was
a struct. 'x == InvalidXLogRecPtr' simply did not work. I don't see much
need for it nowadays. In fact I wonder if we should go in the other
direction and replace XLogRecPtrIsInvalid(x) calls with 'x ==
InvalidXLogRecPtr'.
It's also a bit cumbersome that we have XLogRecPtrIsInvalid() rather
than XLogRecPtrIsValid(). That's inconsistent with OidIsValid and
TransactionIdInValid, and it leads to an awkward double negative 'if
(!XLogRecPtrIsInvalid(x))' if you want to check that 'x' is valid.
Overall I'm inclined to do nothing. But if anything, perhaps introduce
XLogRecPtrIsValid(x) and switch to that, or replace
XLogRecPtrIsInvalid(x) calls with 'x == InvalidXLogRecPtr'
- Heikki
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: hlinnaka@iki.fi, quanzongliang@yeah.net, bertranddrouvot.pg@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: Consistently use the XLogRecPtrIsInvalid() macro
In-Reply-To: <b2ceeea0-abc5-49bb-ac0a-acd84940fc5c@iki.fi>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox