On Sun, Jun 15, 2025 at 7:28 PM vignesh C <vignesh21@gmail.com> wrote:
>
> On Sat, 14 Jun 2025 at 05:22, Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > > > Thank you for updating the patch. I have one comment on the newly added test:
> > > >
> > > > +session "s3"
> > > > +step "s3i1" { INSERT INTO tbl1 (val1, val2) VALUES (1, 1);}
> > > > +step "s3a" { ALTER PUBLICATION pub ADD TABLE tbl1; }
> > > > +step "s3i2" { INSERT INTO tbl1 (val1, val2) VALUES (6, 6); }
> > > > +step "s3_get_binary_changes" { SELECT count(data) FROM
> > > > pg_logical_slot_get_binary_changes('isolation_slot', NULL, NULL,
> > > > 'proto_version', '4', 'publication_names', 'pub') WHERE get_byte(data,
> > > > 0) = 73; }
> > > > +
> > > > +session "s4"
> > > > +step "s4b" { BEGIN; }
> > > > +step "s4i1" { INSERT INTO tbl1 (val1, val2) VALUES (2, 2);}
> > > > +step "s4i2" { INSERT INTO tbl1 (val1, val2) VALUES (4, 4); }
> > > > +step "s4c" { COMMIT; }
> > > > +step "s4i3" { INSERT INTO tbl1 (val1, val2) VALUES (5, 5); }
> > > > +
> > > > +session "s5"
> > > > +step "s5b" { BEGIN; }
> > > > +step "s5i1" { INSERT INTO tbl1 (val1, val2) VALUES (3, 3); }
> > > > +step "s5c" { COMMIT; }
> > > >
> > > > I think we don't necessarily need to add sessions "s4" and "s5". Let's
> > > > reuse "s1" and "s2" instead of adding them. I've attached a patch to
> > > > change that.
> > >
> > > Thanks, this is better.
> > > In the case of PG13 I have slightly changed the test to do the insert
> > > after the commit, because the queuing of invalidation messages into
> > > reorder buffer queue is not supported in PG13. This limitation is due
> > > to the absence of support for REORDER_BUFFER_CHANGE_INVALIDATION which
> > > is already present in >= PG14
> > > The attached v14 version patch has the changes for the same.
> >
> > Hmm, but the modified test is essentially the same as what we already
> > have in invalidation_distribution.spec. I think that it's good to have
> > the same test case in all branches even though we get a different
> > result in v13 as we currently don't have the test to check such
> > differences.
> >
> > I've attached the updated patches accordingly and the commit messages.
>
> Thanks, the changes look good to me.
Pushed the fix (d87d07b7ad3). Thank you for working on this fix.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
