On Mon, Jul 1, 2019 at 1:24 PM Thomas Munro <thomas.mu...@gmail.com> wrote: > > On Fri, Jun 28, 2019 at 6:09 AM Robert Haas <robertmh...@gmail.com> wrote: > > I happened to open up 0001 from this series, which is from Thomas, and > > I do not think that the pg_buffercache changes are correct. The idea > > here is that the customer might install version 1.3 or any prior > > version on an old release, then upgrade to PostgreSQL 13. When they > > do, they will be running with the old SQL definitions and the new > > binaries. At that point, it sure looks to me like the code in > > pg_buffercache_pages.c is going to do the Wrong Thing. [...] > > Yep, that was completely wrong. Here's a new version. >
One comment/question related to 0022-Use-undo-based-rollback-to-clean-up-files-on-abort.patch. +make_undo_smgr_create(RelFileNode *rnode, FullTransactionId fxid, + XLogReaderState *xlog_record) +{ + UnpackedUndoRecord undorecord = {0}; + UndoRecordInsertContext context; + + undorecord.uur_rmid = RM_SMGR_ID; + undorecord.uur_type = UNDO_SMGR_CREATE; + undorecord.uur_info = UREC_INFO_PAYLOAD; + undorecord.uur_dbid = rnode->dbNode; + undorecord.uur_xid = XidFromFullTransactionId(fxid); + undorecord.uur_cid = InvalidCommandId; + undorecord.uur_fork = InvalidForkNumber; While reviewing Dilip's patch(undo-record-interface), I noticed that we include Fork_Num in undo record, if it is not a MAIN_FORKNUM. So, in this patch's case, we will always include it as you are passing InvalidForkNumber. I also see that the patch doesn't use uur_fork in the undo record handler, so I think you don't care what is its value. I am not sure what is the best thing to do here, but it might be better if we can avoiding adding fork_num in each undo record. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com