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