Re: BUG #18055: logical decoding core on AllocateSnapshotBuilder()

Masahiko Sawada <sawada.mshk@gmail.com>

From: Masahiko Sawada <sawada.mshk@gmail.com>
To: Andres Freund <andres@anarazel.de>
Cc: "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>, Amit Kapila <amit.kapila16@gmail.com>, "ocean_li_996@163.com" <ocean_li_996@163.com>, "pgsql-bugs@lists.postgresql.org" <pgsql-bugs@lists.postgresql.org>
Date: 2023-08-21T12:41:46Z
Lists: pgsql-bugs

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Fix uninitialized access to InitialRunningXacts during decoding after ERROR.

On Mon, Aug 21, 2023 at 6:05 AM Andres Freund <andres@anarazel.de> wrote:
>
> Hi,
>
>
> I think the real problem is that 272248a0c added InitialRunningXacts a global
> variable. If it just lived in in struct SnapBuild, this whole thing wouldn't
> be an issue? The commit justified this choice with avoiding an ABI breakage -
> but isn't that bogus? The struct is private to snapbuild.c. It doesn't live on
> disk (that's SnapBuildOnDisk).

No, since SnapBuildOnDisk contains SnapBuild, if we add something to
SnapBuild, the on-disk format compatibility would break. See:

/*
 * We store current state of struct SnapBuild on disk in the following manner:
 *
 * struct SnapBuildOnDisk;
 * TransactionId * running.xcnt_space;
 * TransactionId * committed.xcnt; (*not xcnt_space*)
 *
 */
typedef struct SnapBuildOnDisk
{
    /* first part of this struct needs to be version independent */

    /* data not covered by checksum */
    uint32      magic;
    pg_crc32c   checksum;

    /* data covered by checksum */

    /* version, in case we want to support pg_upgrade */
    uint32      version;
    /* how large is the on disk data, excluding the constant sized part */
    uint32      length;

    /* version dependent part */
    SnapBuild   builder;
:

Regards,

-- 
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com