On Mon, Jul 18, 2022 at 8:49 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Sun, Jul 17, 2022 at 6:29 PM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > > > On Fri, Jul 15, 2022 at 3:32 PM shiy.f...@fujitsu.com > > <shiy.f...@fujitsu.com> wrote: > > > > > > > I've attached patches for all supported branches including the master. > > > > For back branch patches, > * Wouldn't it be better to move purge logic into the function > SnapBuildPurge* function for the sake of consistency?
Agreed. > * Do we really need ReorderBufferInitialXactsSetCatalogChanges()? > Can't we instead have a function similar to > SnapBuildXidHasCatalogChanges() as we have for the master branch? That > will avoid calling it when the snapshot > state is SNAPBUILD_START or SNAPBUILD_BUILDING_SNAPSHOT Seems a good idea. We would need to pass the information about (parsed->xinfo & XACT_XINFO_HAS_INVALS) to the function but probably we can change ReorderBufferXidHasCatalogChanges() so that it checks the RBTXN_HAS_CATALOG_CHANGES flag and then the initial running xacts array. BTW on backbranches, I think that the reason why we add initial_running_xacts stuff to ReorderBuffer is that we cannot modify SnapBuild that could be serialized. Can we add a (private) array for the initial running xacts in snapbuild.c instead of adding new variables to ReorderBuffer? That way, the code would become more consistent with the changes on the master branch. Regards, -- Masahiko Sawada EDB: https://www.enterprisedb.com/
