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

On Mon, Apr 09, 2018 at 03:30:39PM +0300, Heikki Linnakangas wrote:
> 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?

It seems to me that you are right here: those complications are not
necessary.

if (replorigin)
+ {
/* Move LSNs forward for this replication origin */
replorigin_session_advance(replorigin_session_origin_lsn,
gxact->prepare_end_lsn);
+ }
This is better style.

+ /* twophase_gid follows if XINFO_HAS_GID. As a null-terminated string. */
+ /* xl_xact_origin follows if XINFO_HAS_ORIGIN, stored unaligned! */

Worth mentioning that the first one is also unaligned with your patch?
And that all the last fields of xl_xact_commit and xl_xact_abort are
kept as such on purpose?
--
Michael