Message-ID: <20213178-92cd-4ab9-9699-c68cd75bae58@yeah.net>
Date: Tue, 28 Oct 2025 16:53:29 +0800
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Consistently use the XLogRecPtrIsInvalid() macro
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>,
 pgsql-hackers@lists.postgresql.org
References: <aQB7EvGqrbZXrMlg@ip-10-97-1-34.eu-west-3.compute.internal>
Content-Language: en-US
From: Quan Zongliang <quanzongliang@yeah.net>
In-Reply-To: <aQB7EvGqrbZXrMlg@ip-10-97-1-34.eu-west-3.compute.internal>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/20213178-92cd-4ab9-9699-c68cd75bae58%40yeah.net>
Precedence: bulk


On 10/28/25 4:13 PM, Bertrand Drouvot wrote:
> Hi hackers,
> 
> 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.

> I think that's one of the things we could do once a year, like Peter does with
> his annual "clang-tidy" check ([2]).
> 
> Thoughts?
> 
> [1]: https://www.postgresql.org/message-id/CAD21AoB_C6V1PLNs%3DSuOejgGh1o6ZHJMstD7X4X1b_z%3D%3DLdH1Q%40mail.gmail.com
> [2]: https://www.postgresql.org/message-id/CAH2-WzmxPQAF_ZhwrUo3rzVk3yYj_4mqbgiQXAGGO5nFYV3D8Q@mail.gmail.com
> 
> Regards,
>