On Tue, Aug 2, 2022 at 5:31 PM shiy.fnst@fujitsu.com
<shiy.fnst@fujitsu.com> wrote:
>
> On Mon, Aug 1, 2022 10:31 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Mon, Aug 1, 2022 at 7:46 AM Masahiko Sawada
> > <sawada.mshk@gmail.com> wrote:
> > >
> > > On Fri, Jul 29, 2022 at 3:45 PM Amit Kapila <amit.kapila16@gmail.com>
> > wrote:
> > > >
> > >
> > > I've attached updated patches for all branches. Please review them.
> > >
> >
> > Thanks, the patches look mostly good to me. I have made minor edits by
> > removing 'likely' from a few places as those don't seem to be adding
> > much value, changed comments at a few places, and was getting
> > compilation in error in v11/10 (snapbuild.c:2111:3: error: ‘for’ loop
> > initial declarations are only allowed in C99 mode) which I have fixed.
> > See attached, unless there are major comments/suggestions, I am
> > planning to push this day after tomorrow (by Wednesday) after another
> > pass.
> >
>
> Thanks for updating the patch.
>
> Here are some minor comments:
>
> 1.
> patches for REL10 ~ REL13:
> + * Mark the transaction as containing catalog changes. In addition, if the
> + * given xid is in the list of the initial running xacts, we mark the
> + * its subtransactions as well. See comments for NInitialRunningXacts and
> + * InitialRunningXacts for additional info.
>
> "mark the its subtransactions"
> ->
> "mark its subtransactions"
>
> 2.
> patches for REL10 ~ REL15:
> In the comment in catalog_change_snapshot.spec, maybe we can use "RUNNING_XACTS"
> instead of "RUNNING_XACT" "XACT_RUNNING", same as the patch for master branch.
>
Thank you for the comments! These have been incorporated in the latest
version v12 patch I just submitted.
Regards,
--
Masahiko Sawada
EDB: https://www.enterprisedb.com/
