Re: pgsql: Store 2PC GID in commit/abort WAL recs for logical decoding

On 28/03/18 19:47, Simon Riggs wrote:
> Store 2PC GID in commit/abort WAL recs for logical decoding

This forgot to update the comments in xl_xact_commit and xl_xact_abort,
for the new fields.

> +
> + if (parsed->xinfo & XACT_XINFO_HAS_GID)
> + {
> + int gidlen;
> + strcpy(parsed->twophase_gid, data);
> + gidlen = strlen(parsed->twophase_gid) + 1;
> + data += MAXALIGN(gidlen);
> + }
> + }
> +
> + if (parsed->xinfo & XACT_XINFO_HAS_ORIGIN)
> + {
> + xl_xact_origin xl_origin;
> +
> + /* we're only guaranteed 4 byte alignment, so copy onto stack */
> + memcpy(&xl_origin, data, sizeof(xl_origin));
> +
> + parsed->origin_lsn = xl_origin.origin_lsn;
> + parsed->origin_timestamp = xl_origin.origin_timestamp;
> +
> + data += sizeof(xl_xact_origin);
> }

There seems to be some confusion on the padding here. Firstly, what's
the point of zero-padding the GID length to the next MAXALIGN boundary,
which would be 8 bytes on 64-bit systems, if the start is only
guaranteed 4-byte alignment, per the comment at the memcpy() above.
Secondly, if we're memcpying the fields that follow anyway, why bother
with any alignment padding at all?

I propose the attached.

- Heikki