Analysis: Removal of Obsolete Comment in AtEOXact_Inval
Core Issue
This thread is a minor code hygiene cleanup, not a substantive architectural change. The proposal targets a stale documentation comment on AtEOXact_Inval() in src/backend/utils/cache/inval.c that reads:
"This should be called as the last step in processing a transaction."
The comment is factually incorrect and has been for over two decades. AtEOXact_Inval() is invoked from CommitTransaction() and AbortTransaction() in src/backend/access/transam/xact.c, but it is emphatically not the last step — a long tail of end-of-transaction hooks runs after it, including AtEOXact_MultiXact(), AtCommit_Notify(), AtEOXact_GUC(), AtEOXact_SPI(), AtEOXact_Namespace(), AtEOXact_SMgr(), AtEOXact_PgStat(), AtEOXact_Snapshot(), and the logical replication worker cleanups.
Why The Comment Exists (Historical Context)
Daniel Gustafsson's archaeological note is the most interesting technical content in the thread. He traces the comment back to:
- Origin: The comment was originally attached to a function called
RegisterInvalid(), committed by Cimarron Taylor in 1990 — predating the Postgres95 release. - Inheritance: It survived the Postgres95 → PostgreSQL import wholesale, getting carried onto
AtEOXact_Inval()when the invalidation subsystem was refactored. - Invalidation point: Evan Li pinpoints a specific regression of accuracy — the addition of
AtEOXact_MultiXact()roughly 21 years ago (circa 2004–2005, when the multixact infrastructure was built out for shared row locks). That single insertion already falsified the "last step" claim; everything since has piled on.
This is a textbook example of comment rot in a long-lived codebase: an invariant that was true in the original Berkeley POSTGRES design (when invalidation processing genuinely was terminal) silently became false as the transaction commit sequence accreted new concerns (multixact, logical replication, stats, namespace cleanup, etc.).
Why The Ordering Matters Architecturally
Although the comment is wrong about AtEOXact_Inval being last, the position of the call is not arbitrary. The surrounding code in CommitTransaction() carries a still-accurate block comment explaining the real constraint:
Make catalog changes visible to all backends. This has to happen after
relcache references are dropped (see comments for AtEOXact_RelationCache),
but before locks are released (if anyone is waiting for lock on a relation
we've modified, we want them to know about the catalog change before they
start using the relation).
This captures the genuine invariant: invalidation messages must be sent after local relcache cleanup (so we don't re-dirty entries we just dropped) and before lock release (so waiters see the new catalog state when they acquire the lock). The obsolete "last step" note obscures this real ordering constraint by suggesting a stronger — and false — property.
Removing the stale comment therefore has a modest pedagogical benefit: it stops misleading readers about the phase structure of commit processing, and leaves the accurate, nuanced ordering comment at the call site as the authoritative documentation.
Patch Scope and Implications
The patch is a one-line (or small block) deletion in src/backend/utils/cache/inval.c. There are:
- No behavioral changes — pure comment removal.
- No ABI/API implications —
AtEOXact_Inval()'s signature and semantics are untouched. - No test coverage needed — comment-only change.
The risk surface is essentially zero, which is why the thread resolves quickly with a committer simply signaling intent to push.
Disposition
Daniel Gustafsson (committer) declared intent to commit the removal "shortly unless someone disagrees." Given the triviality and unanimous +1 reception, this is a routine cleanup commit. The ~6-month gap between the initial report (October 2025) and Daniel's response (May 2026) reflects the low priority of janitorial patches rather than any technical contention.
Broader Lesson
This thread illustrates a recurring pattern in PostgreSQL maintenance: comments attached to individual functions are fragile when the caller's structure evolves. A comment like "called last" is a statement about the caller, not the callee, and thus belongs (if anywhere) at the call site in xact.c, not in inval.c. The accurate ordering comment already lives at the call site — which is exactly where it should, because that is the code that must be audited if someone proposes reordering commit-time cleanup steps.