Horton

Thread

Commits

  1. Fix incorrect assignment of InvalidXLogRecPtr to a non-LSN variable.

  1. Fix incorrect assignment of InvalidXLogRecPtr to a non-LSN variable.

    Fujii Masao <masao.fujii@gmail.com> — 2025-11-12T08:23:13Z

    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

  2. Re: Fix incorrect assignment of InvalidXLogRecPtr to a non-LSN variable.

    Steven Niu <niushiji@gmail.com> — 2025-11-12T08:47:20Z

    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

  3. Re: Fix incorrect assignment of InvalidXLogRecPtr to a non-LSN variable.

    Xuneng Zhou <xunengzhou@gmail.com> — 2025-11-12T09:14:39Z

    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

  4. Re: Fix incorrect assignment of InvalidXLogRecPtr to a non-LSN variable.

    Fujii Masao <masao.fujii@gmail.com> — 2025-11-12T23:54:10Z

    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