On Tue, Jan 3, 2023 at 10:47 PM Andres Freund <and...@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
