public inbox for [email protected]help / color / mirror / Atom feed
Use XLogRecPtrIsValid() instead of negated XLogRecPtrIsInvalid 9+ messages / 4 participants [nested] [flat]
* Use XLogRecPtrIsValid() instead of negated XLogRecPtrIsInvalid @ 2026-04-10 06:11 vignesh C <[email protected]> 0 siblings, 1 reply; 9+ messages in thread From: vignesh C @ 2026-04-10 06:11 UTC (permalink / raw) To: PostgreSQL Hackers <[email protected]> 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. Regards, Vignesh Attachments: [application/octet-stream] 0001-Remove-double-negative-XLogRecPtr-checks.patch (1.3K, 2-0001-Remove-double-negative-XLogRecPtr-checks.patch) download | inline diff: From 3b8308b9907afa19300d8dbce9ce5794b4319a83 Mon Sep 17 00:00:00 2001 From: Vignesh C <[email protected]> Date: Fri, 10 Apr 2026 10:12:38 +0530 Subject: [PATCH] Remove double-negative XLogRecPtr checks Replace negated XLogRecPtrIsInvalid() checks in init_archive_reader() with XLogRecPtrIsValid(). This improves readability by avoiding double negatives, with no functional change intended. --- src/bin/pg_waldump/archive_waldump.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bin/pg_waldump/archive_waldump.c b/src/bin/pg_waldump/archive_waldump.c index e4a4bf44a7e..ff4c9276d31 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 ^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Use XLogRecPtrIsValid() instead of negated XLogRecPtrIsInvalid @ 2026-04-10 14:46 Fujii Masao <[email protected]> parent: vignesh C <[email protected]> 0 siblings, 1 reply; 9+ messages in thread From: Fujii Masao @ 2026-04-10 14:46 UTC (permalink / raw) To: vignesh C <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]> 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? Regards, [1] $ git grep -E "[=\!]= InvalidXLogRecPtr" src/backend/commands/repack_worker.c: Assert(ctx->reader->EndRecPtr != InvalidXLogRecPtr); src/backend/replication/walreceiver.c: applyPtr = (latestApplyPtr == InvalidXLogRecPtr) ? -- Fujii Masao ^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Use XLogRecPtrIsValid() instead of negated XLogRecPtrIsInvalid @ 2026-04-13 07:10 vignesh C <[email protected]> parent: Fujii Masao <[email protected]> 0 siblings, 1 reply; 9+ messages in thread From: vignesh C @ 2026-04-13 07:10 UTC (permalink / raw) To: Fujii Masao <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]> 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 ^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Use XLogRecPtrIsValid() instead of negated XLogRecPtrIsInvalid @ 2026-04-16 04:11 Fujii Masao <[email protected]> parent: vignesh C <[email protected]> 0 siblings, 2 replies; 9+ messages in thread From: Fujii Masao @ 2026-04-16 04:11 UTC (permalink / raw) To: vignesh C <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]> On Mon, Apr 13, 2026 at 4:10 PM vignesh C <[email protected]> wrote: > I felt these also should be updated, the attached v2 version patch > includes the changes for the same. Thanks for updating the patch! - applyPtr = (latestApplyPtr == InvalidXLogRecPtr) ? + applyPtr = (XLogRecPtrIsInvalid(latestApplyPtr)) ? XLogRecPtrIsValid() should be used here, instead? Regards, -- Fujii Masao ^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Use XLogRecPtrIsValid() instead of negated XLogRecPtrIsInvalid @ 2026-04-16 05:27 Xiaopeng Wang <[email protected]> parent: Fujii Masao <[email protected]> 1 sibling, 0 replies; 9+ messages in thread From: Xiaopeng Wang @ 2026-04-16 05:27 UTC (permalink / raw) To: Fujii Masao <[email protected]>; vignesh C <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]> 在 2026/4/16 12:11, Fujii Masao 写道: > On Mon, Apr 13, 2026 at 4:10 PM vignesh C <[email protected]> wrote: >> I felt these also should be updated, the attached v2 version patch >> includes the changes for the same. > Thanks for updating the patch! > > - applyPtr = (latestApplyPtr == InvalidXLogRecPtr) ? > + applyPtr = (XLogRecPtrIsInvalid(latestApplyPtr)) ? > > XLogRecPtrIsValid() should be used here, instead? > > Regards, > Yeah, I was about to raise the same comment, and Fujii-san beat me. I believe it should be XLogRecPtrIsValid(). Otherwise, the patch looks good to me. Regards, Xiaopeng Wang ^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Use XLogRecPtrIsValid() instead of negated XLogRecPtrIsInvalid @ 2026-04-16 10:55 vignesh C <[email protected]> parent: Fujii Masao <[email protected]> 1 sibling, 1 reply; 9+ messages in thread From: vignesh C @ 2026-04-16 10:55 UTC (permalink / raw) To: Fujii Masao <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]> On Thu, 16 Apr 2026 at 09:42, Fujii Masao <[email protected]> wrote: > > On Mon, Apr 13, 2026 at 4:10 PM vignesh C <[email protected]> wrote: > > I felt these also should be updated, the attached v2 version patch > > includes the changes for the same. > > Thanks for updating the patch! > > - applyPtr = (latestApplyPtr == InvalidXLogRecPtr) ? > + applyPtr = (XLogRecPtrIsInvalid(latestApplyPtr)) ? > > XLogRecPtrIsValid() should be used here, instead? Yes, that seems better, the attached v3 version patch has the changes for the same. Regards, Vignesh Attachments: [application/octet-stream] v3-0001-Use-XLogRecPtr-validity-helper-macros-consistentl.patch (2.8K, 2-v3-0001-Use-XLogRecPtr-validity-helper-macros-consistentl.patch) download | inline diff: From 4da2338268f0c7524a2ff96c12c598512b3e019c Mon Sep 17 00:00:00 2001 From: Vignesh C <[email protected]> Date: Fri, 10 Apr 2026 10:12:38 +0530 Subject: [PATCH v3] 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 | 4 ++-- src/bin/pg_waldump/archive_waldump.c | 4 ++-- 3 files changed, 5 insertions(+), 5 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 5ee6431091e..5eab87625db 100644 --- a/src/backend/replication/walreceiver.c +++ b/src/backend/replication/walreceiver.c @@ -1169,8 +1169,8 @@ XLogWalRcvSendReply(bool force, bool requestReply, bool checkApply) /* Construct a new message */ writePtr = LogstreamResult.Write; flushPtr = LogstreamResult.Flush; - applyPtr = (latestApplyPtr == InvalidXLogRecPtr) ? - GetXLogReplayRecPtr(NULL) : latestApplyPtr; + applyPtr = (XLogRecPtrIsValid(latestApplyPtr)) ? + latestApplyPtr : GetXLogReplayRecPtr(NULL); resetStringInfo(&reply_message); pq_sendbyte(&reply_message, PqReplMsg_StandbyStatusUpdate); 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 ^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Use XLogRecPtrIsValid() instead of negated XLogRecPtrIsInvalid @ 2026-04-16 12:30 Amul Sul <[email protected]> parent: vignesh C <[email protected]> 0 siblings, 1 reply; 9+ messages in thread From: Amul Sul @ 2026-04-16 12:30 UTC (permalink / raw) To: vignesh C <[email protected]>; +Cc: Fujii Masao <[email protected]>; PostgreSQL Hackers <[email protected]> On Thu, Apr 16, 2026 at 4:25 PM vignesh C <[email protected]> wrote: > > On Thu, 16 Apr 2026 at 09:42, Fujii Masao <[email protected]> wrote: > > > > On Mon, Apr 13, 2026 at 4:10 PM vignesh C <[email protected]> wrote: > > > I felt these also should be updated, the attached v2 version patch > > > includes the changes for the same. > > > > Thanks for updating the patch! > > > > - applyPtr = (latestApplyPtr == InvalidXLogRecPtr) ? > > + applyPtr = (XLogRecPtrIsInvalid(latestApplyPtr)) ? > > > > XLogRecPtrIsValid() should be used here, instead? The outer parentheses do not seem to be needed, as XLogRecPtrIsInvalid() already includes them. Other than that, the v3 patch looks good to me. Regards, Amul ^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Use XLogRecPtrIsValid() instead of negated XLogRecPtrIsInvalid @ 2026-04-16 14:06 Fujii Masao <[email protected]> parent: Amul Sul <[email protected]> 0 siblings, 1 reply; 9+ messages in thread From: Fujii Masao @ 2026-04-16 14:06 UTC (permalink / raw) To: Amul Sul <[email protected]>; +Cc: vignesh C <[email protected]>; PostgreSQL Hackers <[email protected]> On Thu, Apr 16, 2026 at 9:31 PM Amul Sul <[email protected]> wrote: > The outer parentheses do not seem to be needed, as > XLogRecPtrIsInvalid() already includes them. > > Other than that, the v3 patch looks good to me. I've updated the patch based on Amul's suggestion and pushed it. Thanks! Regards, -- Fujii Masao ^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Use XLogRecPtrIsValid() instead of negated XLogRecPtrIsInvalid @ 2026-04-16 14:12 vignesh C <[email protected]> parent: Fujii Masao <[email protected]> 0 siblings, 0 replies; 9+ messages in thread From: vignesh C @ 2026-04-16 14:12 UTC (permalink / raw) To: Fujii Masao <[email protected]>; +Cc: Amul Sul <[email protected]>; PostgreSQL Hackers <[email protected]> On Thu, 16 Apr 2026 at 19:36, Fujii Masao <[email protected]> wrote: > > On Thu, Apr 16, 2026 at 9:31 PM Amul Sul <[email protected]> wrote: > > The outer parentheses do not seem to be needed, as > > XLogRecPtrIsInvalid() already includes them. > > > > Other than that, the v3 patch looks good to me. > > I've updated the patch based on Amul's suggestion and pushed it. Thanks! Thanks for pushing the patch. Regards, Vignesh ^ permalink raw reply [nested|flat] 9+ messages in thread
end of thread, other threads:[~2026-04-16 14:12 UTC | newest] Thread overview: 9+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2026-04-10 06:11 Use XLogRecPtrIsValid() instead of negated XLogRecPtrIsInvalid vignesh C <[email protected]> 2026-04-10 14:46 ` Fujii Masao <[email protected]> 2026-04-13 07:10 ` vignesh C <[email protected]> 2026-04-16 04:11 ` Fujii Masao <[email protected]> 2026-04-16 05:27 ` Xiaopeng Wang <[email protected]> 2026-04-16 10:55 ` vignesh C <[email protected]> 2026-04-16 12:30 ` Amul Sul <[email protected]> 2026-04-16 14:06 ` Fujii Masao <[email protected]> 2026-04-16 14:12 ` vignesh C <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox