Re: Consistently use the XLogRecPtrIsInvalid() macro

Quan Zongliang <quanzongliang@yeah.net>

From: Quan Zongliang <quanzongliang@yeah.net>

To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>, pgsql-hackers@lists.postgresql.org

Date: 2025-10-28T08:53:29Z

Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

Replace literal 0 with InvalidXLogRecPtr for XLogRecPtr assignments
- ec3174407164 19 (unreleased) landed
Replace pointer comparisons and assignments to literal zero with NULL
- ec782f56b0c3 19 (unreleased) landed
Use XLogRecPtrIsValid() in various places
- a2b02293bc65 19 (unreleased) landed
Introduce XLogRecPtrIsValid()
- d2965f627fe3 14.20 landed
- c0031d461324 18.1 landed
- 723cc84db50a 16.11 landed
- 49b45999f3b2 15.15 landed
- 33727aff18d0 17.7 landed
- 20bafb097288 13.23 landed
- 06edbed47862 19 (unreleased) landed


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,
>