public inbox for [email protected]  
help / color / mirror / Atom feed
From: vignesh C <[email protected]>
To: Fujii Masao <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Use XLogRecPtrIsValid() instead of negated XLogRecPtrIsInvalid
Date: Mon, 13 Apr 2026 12:40:06 +0530
Message-ID: <CALDaNm0zJuUon52OEx5SpMoP8LEGJh+=PyYqwmcD7Zh97_dnJg@mail.gmail.com> (raw)
In-Reply-To: <CAHGQGwEuEd2StEYuP9vj+jCFT=hQbX8xcQeq8=Kj8r9xT3vmyg@mail.gmail.com>
References: <CALDaNm16knMFtcqyAG3XYSkyagmVXfhaR0T=hau8UTAU0+eLQQ@mail.gmail.com>
	<CAHGQGwEuEd2StEYuP9vj+jCFT=hQbX8xcQeq8=Kj8r9xT3vmyg@mail.gmail.com>

On Fri, 10 Apr 2026 at 20:16, Fujii Masao <[email protected]> wrote:
>
> On Fri, Apr 10, 2026 at 3:11 PM vignesh C <[email protected]> wrote:
> >
> > Hi,
> >
> > This small cleanup patch updates src/bin/pg_waldump/archive_waldump.c
> > to use the recently introduced XLogRecPtrIsValid() helper instead of
> > negating XLogRecPtrIsInvalid(). The current code uses double-negative
> > checks such as:
> > Assert(!XLogRecPtrIsInvalid(privateInfo->startptr));
> > if (!XLogRecPtrIsInvalid(privateInfo->endptr))
> >
> > This patch changes them to:
> > Assert(XLogRecPtrIsValid(privateInfo->startptr));
> > if (XLogRecPtrIsValid(privateInfo->endptr))
> >
> > This improves readability without changing behavior. The attached
> > patch has the changes for the same.
>
> Commit a2b02293bc6 switched various places to use XLogRecPtrIsValid(),
> but it looks like later commits accidentally introduced uses of
> XLogRecPtrIsInvalid() again. So +1 for this change.
>
> Also, that commit replaced direct comparisons with InvalidXLogRecPtr with
> XLogRecPtrIsValid(). I noticed two such comparisons [1]. Should these be
> updated as well?

I felt these also should be updated, the attached v2 version patch
includes the changes for the same.

Regards,
Vignesh


Attachments:

  [application/octet-stream] v2-0001-Use-XLogRecPtr-validity-helper-macros-consistentl.patch (2.7K, 2-v2-0001-Use-XLogRecPtr-validity-helper-macros-consistentl.patch)
  download | inline diff:
From a0fd1f60491a315c7458468a8157a6f79dd384de Mon Sep 17 00:00:00 2001
From: Vignesh C <[email protected]>
Date: Fri, 10 Apr 2026 10:12:38 +0530
Subject: [PATCH v2] Use XLogRecPtr validity helper macros consistently

Replace direct comparisons against InvalidXLogRecPtr and negated
XLogRecPtrIsInvalid() checks with the corresponding
XLogRecPtrIsValid()/XLogRecPtrIsInvalid() helper macros.

This improves readability and consistency by using the dedicated
XLogRecPtr validity helpers throughout the code, with no intended
behavioral change.
---
 src/backend/commands/repack_worker.c  | 2 +-
 src/backend/replication/walreceiver.c | 2 +-
 src/bin/pg_waldump/archive_waldump.c  | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c
index 5bd020e0184..362e0766c41 100644
--- a/src/backend/commands/repack_worker.c
+++ b/src/backend/commands/repack_worker.c
@@ -268,7 +268,7 @@ repack_setup_logical_decoding(Oid relid)
 	ctx->reader->routine.page_read = read_local_xlog_page_no_wait;
 
 	/* Some WAL records should have been read. */
-	Assert(ctx->reader->EndRecPtr != InvalidXLogRecPtr);
+	Assert(XLogRecPtrIsValid(ctx->reader->EndRecPtr));
 
 	/*
 	 * Initialize repack_current_segment so that we can notice WAL segment
diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c
index 09fde92bfd7..01362fd3e95 100644
--- a/src/backend/replication/walreceiver.c
+++ b/src/backend/replication/walreceiver.c
@@ -1169,7 +1169,7 @@ XLogWalRcvSendReply(bool force, bool requestReply, bool checkApply)
 	/* Construct a new message */
 	writePtr = LogstreamResult.Write;
 	flushPtr = LogstreamResult.Flush;
-	applyPtr = (latestApplyPtr == InvalidXLogRecPtr) ?
+	applyPtr = (XLogRecPtrIsInvalid(latestApplyPtr)) ?
 		GetXLogReplayRecPtr(NULL) : latestApplyPtr;
 
 	resetStringInfo(&reply_message);
diff --git a/src/bin/pg_waldump/archive_waldump.c b/src/bin/pg_waldump/archive_waldump.c
index 78b03bc1f85..79915c0a0ce 100644
--- a/src/bin/pg_waldump/archive_waldump.c
+++ b/src/bin/pg_waldump/archive_waldump.c
@@ -216,11 +216,11 @@ init_archive_reader(XLogDumpPrivate *privateInfo,
 	 * With the WAL segment size available, we can now initialize the
 	 * dependent start and end segment numbers.
 	 */
-	Assert(!XLogRecPtrIsInvalid(privateInfo->startptr));
+	Assert(XLogRecPtrIsValid(privateInfo->startptr));
 	XLByteToSeg(privateInfo->startptr, privateInfo->start_segno,
 				privateInfo->segsize);
 
-	if (!XLogRecPtrIsInvalid(privateInfo->endptr))
+	if (XLogRecPtrIsValid(privateInfo->endptr))
 		XLByteToSeg(privateInfo->endptr, privateInfo->end_segno,
 					privateInfo->segsize);
 
-- 
2.43.0



view thread (9+ messages)  latest in thread

reply

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: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: Use XLogRecPtrIsValid() instead of negated XLogRecPtrIsInvalid
  In-Reply-To: <CALDaNm0zJuUon52OEx5SpMoP8LEGJh+=PyYqwmcD7Zh97_dnJg@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox