Sorry for the long delay... On Thu, Jul 4, 2019 at 5:25 PM Julien Rouhaud <rjuju...@gmail.com> wrote: > > On Thu, Jul 4, 2019 at 9:45 AM Michael Paquier <mich...@paquier.xyz> wrote: > > > > On Wed, Jul 03, 2019 at 08:23:44PM +0200, Julien Rouhaud wrote: > > > So the patch compiles and works as intended. I don't have much to say > > > about it, it all looks good to me, since the concerns about xactdesc.c > > > aren't worth the trouble. > > > > > > I'm not sure that I understand Michael's objection though, as > > > xl_xact_prepare is not a new definition and AFAICS it couldn't contain > > > the records anyway. So I'll let him say if he has further objections > > > or if it's ready for committer! > > > > This patch provides parsing information only for the header of the 2PC > > record. Wouldn't it be interesting to get more information from the > > various TwoPhaseRecordOnDisk's callbacks? We could also print much > > more information in xact_desc_prepare(). Like the subxacts, the XID, > > the invalidation messages and the delete-on-abort/commit rels. > > Most of those are already described in the COMMIT PREPARE message, > wouldn't that be redundant? abortrels aren't displayed anywhere > though, so +1 for adding them.
xact_desc_abort() for ROLLBACK PREPARED describes abortrels. No? > I also see that the dbid isn't displayed in any of the 2PC message, > that'd be useful to have it directly instead of looking for it in > other messages for the same transaction. dbid is not reported even in COMMIT message. So I don't like adding dbid into only the PREPARE message. Regards, -- Fujii Masao
