On Thu, Apr 25, 2019 at 03:08:36PM -0400, Alvaro Herrera wrote:
> On 2019-Apr-26, Fujii Masao wrote:
>> I did that arrangement because the format of PREPARE TRANSACTION record,
>> i.e., that struct, also needs to be accessed in backend and frontend.
>> But, of course, if there is smarter way, I'm happy to adopt that!
>
> I don't know. I spent some time staring at the code involved, and it
> seems it'd be possible to improve just a little bit on cleanliness
> grounds, with a lot of effort, but not enough practical value.
Describing those records is something we should do. There are other
parsing routines in xactdesc.c for commit and abort records, so having
that extra routine for prepare at the same place does not sound
strange to me.
+typedef xl_xact_prepare TwoPhaseFileHeader;
I find this mapping implementation a bit lazy, and your
newly-introduced xl_xact_prepare does not count for all the contents
of the actual WAL record for PREPARE TRANSACTION. Wouldn't it be
better to put all the contents of the record in the same structure,
and not only the 2PC header information?
This is not v12 material of course.
--
Michael