On Wed, Jul 20, 2022 at 9:01 AM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > On Wed, Jul 20, 2022 at 12:11 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > On Tue, Jul 19, 2022 at 7:28 PM Masahiko Sawada <sawada.m...@gmail.com> > > wrote: > > > > Why do you think we can't remove > > ReorderBufferInitialXactsSetCatalogChanges() from the back branch > > patch? I think we don't need to change the existing function > > ReorderBufferXidHasCatalogChanges() but instead can have a wrapper > > like SnapBuildXidHasCatalogChanges() similar to master branch patch. > > IIUC we need to change SnapBuildCommitTxn() but it's exposed. > > Currently, we call like DecodeCommit() -> SnapBuildCommitTxn() -> > ReorderBufferXidHasCatalogChanges(). If we have a wrapper function, we > call like DecodeCommit() -> SnapBuildCommitTxn() -> > SnapBuildXidHasCatalogChanges() -> > ReorderBufferXidHasCatalogChanges(). In > SnapBuildXidHasCatalogChanges(), we need to check if the transaction > has XACT_XINFO_HAS_INVALS, which means DecodeCommit() needs to pass > either parsed->xinfo or (parsed->xinfo & XACT_XINFO_HAS_INVALS != 0) > down to SnapBuildXidHasCatalogChanges(). However, since > SnapBuildCommitTxn(), between DecodeCommit() and > SnapBuildXidHasCatalogChanges(), is exposed we cannot change it. >
Agreed. > Another idea would be to have functions, say > SnapBuildCommitTxnWithXInfo() and SnapBuildCommitTxn_ext(). The latter > does actual work of handling transaction commits and both > SnapBuildCommitTxn() and SnapBuildCommit() call > SnapBuildCommitTxnWithXInfo() with different arguments. > Do you want to say DecodeCommit() instead of SnapBuildCommit() in above para? Yet another idea could be to have another flag RBTXN_HAS_INVALS which will be set by DecodeCommit for top-level TXN. Then, we can retrieve it even for each of the subtxn's if and when required. -- With Regards, Amit Kapila.
