On Wed, Oct 19, 2022 at 9:40 AM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > I've attached patches for Change-3 with a test case. Please review them as > well. >
The patch looks mostly good to me apart from few minor comments which are as follows: 1. +# The last decoding restarts from the first checkpoint, and add invalidation messages +# generated by "s0_truncate" to the subtransaction. When decoding the commit record of +# the top-level transaction, we mark both top-level transaction and its subtransactions +# as containing catalog changes. However, we check if we don't create the association +# between top-level and subtransactions at this time. Otherwise, we miss executing +# invalidation messages when forgetting the transaction. +permutation "s0_init" "s0_begin" "s0_savepoint" "s0_insert" "s1_checkpoint" "s1_get_changes" "s0_truncate" "s0_commit" "s0_begin" "s0_insert" "s1_checkpoint" "s1_get_changes" "s0_commit" "s1_get_changes" The second part of this comment seems to say things more than required which makes it less clear. How about something like: "The last decoding restarts from the first checkpoint and adds invalidation messages generated by "s0_truncate" to the subtransaction. While processing the commit record for the top-level transaction, we decide to skip this xact but ensure that corresponding invalidation messages get processed."? 2. + /* + * We will assign subtransactions to the top transaction before + * replaying the contents of the transaction. + */ I don't think we need this comment. -- With Regards, Amit Kapila.
