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
signature.asc
Description: PGP signature