On Thu, Jul 6, 2023 at 2:06 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Wed, Jul 5, 2023 at 7:20 PM Ashutosh Bapat > <ashutosh.bapat....@gmail.com> wrote: > > > > On Wed, Jul 5, 2023 at 2:29 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > > > On Wed, Jul 5, 2023 at 2:28 PM Amit Kapila <amit.kapil...@gmail.com> > > > wrote: > > > > > > > > On Mon, Jul 3, 2023 at 4:49 PM vignesh C <vignes...@gmail.com> wrote: > > > > > > > > > > +1 for the first version patch, I also felt the first version is > > > > > easily understandable. > > > > > > > > > > > > > Okay, please find the slightly updated version (changed a comment and > > > > commit message). Unless there are more comments, I'll push this in a > > > > day or two. > > > > > > > > > > oops, forgot to attach the patch. > > > > I still think that we need to do something so that a new call to > > pg_output_begin() automatically takes care of the conditions under > > which it should be called. Otherwise, we will introduce a similar > > problem in some other place in future. > > > > AFAIU, this problem is because we forget to conditionally call > pg_output_begin() from pg_decode_message() which can happen with or > without moving conditions inside pg_output_begin(). Also, it shouldn't > be done at the expense of complexity. I find the check added by > Vignesh's v2 patch (+ if (!(last_write ^ data->skip_empty_xacts) || > txndata->xact_wrote_changes)) a bit difficult to understand and more > error-prone. The others like Hou-San also couldn't understand unless > Vignesh gave an explanation. I also thought of avoiding that check. > Basically, IIUC, the check is added because the patch also removed > 'data->skip_empty_xacts' check from > pg_decode_begin_txn()/pg_decode_begin_prepare_txn(). Now, if retain > those checks then it is probably okay but again the similar checks are > still split and that doesn't appear to be better than the v1 or v3 > patch version. I am not against improving code in this area and > probably we can consider doing it as a separate patch if we have > better ideas instead of combining it with this patch. >
I have pushed this work. But feel free to propose further improvements, if you have any better ideas. -- With Regards, Amit Kapila.