On Mon, Jul 11, 2022 9:54 PM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > I've attached an updated patch, please review it. >
Thanks for your patch. Here are some comments for the REL14-v1 patch. 1. + Size sz = sizeof(TransactionId) * nxacts;; There is a redundant semicolon at the end. 2. + workspace = MemoryContextAlloc(rb->context, rb->n_initial_running_xacts); Should it be: + workspace = MemoryContextAlloc(rb->context, sizeof(TransactionId) * rb->n_initial_running_xacts); 3. + /* bound check if there is at least one transaction to be removed */ + if (NormalTransactionIdPrecedes(rb->initial_running_xacts[0], + running->oldestRunningXid)) + return; + Here, I think it should return if rb->initial_running_xacts[0] is older than oldestRunningXid, right? Should it be changed to: + if (!NormalTransactionIdPrecedes(rb->initial_running_xacts[0], + running->oldestRunningXid)) + return; 4. + if ((parsed->xinfo & XACT_XINFO_HAS_INVALS) != 0) Maybe we can change it like the following, to be consistent with other places in this file. It's also fine if you don't change it. + if (parsed->xinfo & XACT_XINFO_HAS_INVALS) Regards, Shi yu
