Fix incorrect assignment of InvalidXLogRecPtr to a non-LSN variable.
Hi,
I noticed that pg_logical_slot_get_changes_guts() assigns InvalidXLogRecPtr
to the local variable upto_nchanges, even though it's not LSN variable
(i.e., its type is int32, not XLogRecPtr). While this causes no functional issue
since InvalidXLogRecPtr is defined as 0, it's semantically incorrect.
I propose fixing this by setting upto_nchanges to 0 instead of
InvalidXLogRecPtr.
Attached is a patch implementing this change.
Regards,
--
Fujii Masao
Attachments:
v1-0001-Fix-incorrect-assignment-of-InvalidXLogRecPtr-to-.patchapplication/octet-stream; name=v1-0001-Fix-incorrect-assignment-of-InvalidXLogRecPtr-to-.patchDownload
From 319d5dae6f073c3e1ecea79d51aefdad6ecaa884 Mon Sep 17 00:00:00 2001
From: Fujii Masao <fujii@postgresql.org>
Date: Wed, 12 Nov 2025 17:10:35 +0900
Subject: [PATCH v1] Fix incorrect assignment of InvalidXLogRecPtr to a non-LSN
variable.
pg_logical_slot_get_changes_guts() previously assigned InvalidXLogRecPtr to
the local variable upto_nchanges, which is of type int32, not XLogRecPtr.
While this caused no functional issue since InvalidXLogRecPtr is defined as 0,
it was semantically incorrect.
This commit fixes the issue by updating pg_logical_slot_get_changes_guts()
to set upto_nchanges to 0 instead of InvalidXLogRecPtr.
No backpatch is needed, as the previous behavior was harmless.
---
src/backend/replication/logical/logicalfuncs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/backend/replication/logical/logicalfuncs.c b/src/backend/replication/logical/logicalfuncs.c
index 49b2aef3c74..25b2b795925 100644
--- a/src/backend/replication/logical/logicalfuncs.c
+++ b/src/backend/replication/logical/logicalfuncs.c
@@ -129,7 +129,7 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin
upto_lsn = PG_GETARG_LSN(1);
if (PG_ARGISNULL(2))
- upto_nchanges = InvalidXLogRecPtr;
+ upto_nchanges = 0;
else
upto_nchanges = PG_GETARG_INT32(2);
--
2.51.2
Agreed.
The definitions of upto_lsn and upyo_nchanges are different, so they should not be assigned the same form of value.
XLogRecPtr upto_lsn;
int32 upto_nchanges;
......
if (PG_ARGISNULL(1))
upto_lsn = InvalidXLogRecPtr;
else
upto_lsn = PG_GETARG_LSN(1);
if (PG_ARGISNULL(2))
upto_nchanges = InvalidXLogRecPtr;
else
upto_nchanges = PG_GETARG_INT32(2);
Thanks,
Steven
________________________________
From: Fujii Masao <masao.fujii@gmail.com>
Sent: Wednesday, November 12, 2025 16:23
To: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Fix incorrect assignment of InvalidXLogRecPtr to a non-LSN variable.
Hi,
I noticed that pg_logical_slot_get_changes_guts() assigns InvalidXLogRecPtr
to the local variable upto_nchanges, even though it's not LSN variable
(i.e., its type is int32, not XLogRecPtr). While this causes no functional issue
since InvalidXLogRecPtr is defined as 0, it's semantically incorrect.
I propose fixing this by setting upto_nchanges to 0 instead of
InvalidXLogRecPtr.
Attached is a patch implementing this change.
Regards,
--
Fujii Masao
Hi,
On Wed, Nov 12, 2025 at 4:23 PM Fujii Masao <masao.fujii@gmail.com> wrote:
Hi,
I noticed that pg_logical_slot_get_changes_guts() assigns InvalidXLogRecPtr
to the local variable upto_nchanges, even though it's not LSN variable
(i.e., its type is int32, not XLogRecPtr). While this causes no functional issue
since InvalidXLogRecPtr is defined as 0, it's semantically incorrect.I propose fixing this by setting upto_nchanges to 0 instead of
InvalidXLogRecPtr.
Attached is a patch implementing this change.Regards,
--
Fujii Masao
Good catch! I checked that no other similar misuses of
InvalidXLogRecPtr assigned to non-LSN variables were found in
logicalfuncs.c.
--
Best,
Xuneng
On Wed, Nov 12, 2025 at 6:14 PM Xuneng Zhou <xunengzhou@gmail.com> wrote:
Good catch! I checked that no other similar misuses of
InvalidXLogRecPtr assigned to non-LSN variables were found in
logicalfuncs.c.
Thanks, Steven and Xuneng, for the reviews! I've pushed the patch.
Regards,
--
Fujii Masao