On Tue, Jan 3, 2023 at 10:47 PM Andres Freund <andres@anarazel.de> wrote:
> IMO the comment at the top mentioning why the TransactionIdIsInProgress()
> calls are crucial / need to be done first would be considerably more likely to
> be found in transam.c than heapam_visibility.c.
Yeah, but they're duplicated anyway. For example in the transam
README. Plus we have references to these same comments from other
files, such as heapam.c, which mentions heapam_visibility.c by name as
where you go to learn more about this issue.
> And it'd make sense to have
> the explanation of why TransactionIdDidAbort() isn't the same as
> !TransactionIdDidCommit(), even for !TransactionIdIsInProgress() xacts, near
> the explanation for doing TransactionIdIsInProgress() first.
I think that we should definitely have a comment directly over
TransactionIdDidAbort(). Though I wouldn't mind reorganizing these
other comments, or making the comment over TransactionIdDidAbort()
mostly just point to the other comments.
--
Peter Geoghegan