On Wed, Sep 24, 2025 at 10:23 PM Matheus Alcantara
<matheusssilv97@gmail.com> wrote:
>
> On Wed Sep 24, 2025 at 10:59 AM -03, Arseniy Mukhin wrote:
> > ...
> > Patch looks great. Some minor points:
> >
> > I have a warning when using git am with the patch:
> > warning: 1 line adds whitespace errors.
> >
> I don't think that this is a problem? And I don't know how to remove
> this warning, I've just created the patch with git format-patch @~1
>
In my experience a run of pgindent usually fixes all such warnings,
but I agree it's not a big deal.
>
> > What do you think about a Assert in asyncQueueRollbackNotifications()
> > that other backends still see us as 'in progress'? So we can be sure
> > that they can't process our notifications before we mark notifications
> > as 'committed=false'. Not sure how to do it correctly, maybe
> >
> > Assert(TransactionIdIsValid(MyProc->xid));
> >
> > will work? The TAP test that I tried to write also should test it.
> >
> Sounds resanable to me. I think that we could use the following assert
> when iterating over the entries, what do you think?
> Assert(TransactionIdIsInProgress(qe->xid));
>
Yeah, TransactionIdIsInProgress() looks like the right way to do it.
And one more 'Assert' idea... maybe we can add
Assert(TransactionIdIsCurrentTransactionId(qe->xid)) just to verify
that we are only touching our own notifications?
Thank you, the fixes look good.
Best regards,
Arseniy Mukhin
