Remove unused isCommit parameter from AtEOXact_LocalBuffers
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t53243psql -h localhost -U postgresBuilt from patchset v1 (message #1), September 20, 2026 at 01:39 AM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t53243_1 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t53243_1 && git checkout t53243_1Patchset v1 (message #1) is on t53243_1
Hi all,
I see the `isCommit` parameter of `AtEOXact_LocalBuffers` is unused:
```
void
AtEOXact_LocalBuffers(bool isCommit)
{
CheckForLocalBufferLeaks();
}
```
Commit 5df307c7782518c4a3c19ffd05c7cb591b97e23c introduced `AtEOXact_LocalBuffers(bool isCommit)` for the following logic:
```
if (isCommit)
elog(WARNING,
"local buffer leak: [%03d] (rel=%u/%u/%u, blockNum=%u, flags=0x%x, refcount=%u %d)",
i,
buf->tag.rnode.spcNode, buf->tag.rnode.dbNode,
buf->tag.rnode.relNode, buf->tag.blockNum, buf->flags,
buf->refcount, LocalRefCount[i]);
```
Commit fdd13f156814f81732c188788ab1b7b14c59f4da removed the above code, but the `isCommit` parameter was left intact.
I intend to remove this unused parameter, and also clean up the call site in `AtEOXact_Buffers` since it invokes `AtEOXact_LocalBuffers`.
The code contains multiple calls to `AtEOXact_Buffers`, some of which pass `false` and others pass `true`, which may be confusing.
Attached is the patch for these changes.
--
regards,
Man Zeng
"=?ISO-8859-1?B?emVuZ21hbg==?=" <zengman@halodbtech.com> writes:
Commit fdd13f156814f81732c188788ab1b7b14c59f4da removed the above code, but the `isCommit` parameter was left intact.
I intend to remove this unused parameter, and also clean up the call site in `AtEOXact_Buffers` since it invokes `AtEOXact_LocalBuffers`.
I think we should reject this as useless code churn. The parameter
was needed in the past and might be needed again in the future.
It's fairly common for other AtEOXact functions to take an isCommit
flag, so I don't find it surprising for these to have one.
regards, tom lane
On Tue, Feb 03, 2026 at 09:45:49AM -0500, Tom Lane wrote:
I think we should reject this as useless code churn. The parameter
was needed in the past and might be needed again in the future.
It's fairly common for other AtEOXact functions to take an isCommit
flag, so I don't find it surprising for these to have one.
+1
--
nathan
On Tue, Feb 03, 2026 at 09:34:33AM -0600, Nathan Bossart wrote:
On Tue, Feb 03, 2026 at 09:45:49AM -0500, Tom Lane wrote:
I think we should reject this as useless code churn. The parameter
was needed in the past and might be needed again in the future.
It's fairly common for other AtEOXact functions to take an isCommit
flag, so I don't find it surprising for these to have one.+1
+1. Symmetry is usually relevant in the signature of these cleanup
functions.
--
MichaelI think we should reject this as useless code churn. The parameter
was needed in the past and might be needed again in the future.
It's fairly common for other AtEOXact functions to take an isCommit
flag, so I don't find it surprising for these to have one.+1
+1. Symmetry is usually relevant in the signature of these cleanup
functions.
Hi all,
I still want to do this simple cleanup, and I’d like to state my thoughts:
1. It has been a very long time, about more than twenty years, since the `isCommit` parameter of `AtEOXact_LocalBuffers` last functioned, namely commit fdd13f156814f81732c188788ab1b7b14c59f4da.
2. This modification mainly involves `AtEOXact_Buffers`. Judging from the comment, the work undertaken by this function seems simple and stable.
```
/*
* AtEOXact_Buffers - clean up at end of transaction.
*
* As of PostgreSQL 8.0, buffer pins should get released by the
* ResourceOwner mechanism. This routine is just a debugging
* cross-check that no pins remain.
*/
void
AtEOXact_Buffers(bool isCommit)
{
CheckForBufferLeaks();
AtEOXact_LocalBuffers(isCommit);
Assert(PrivateRefCountOverflowed == 0);
}
```
3. Some other AtEOXact functions take no parameters, although this is not a strong argument here.
However, I understand everyone's thoughts and respect your opinions, so I choose to keep my own opinion on this.
--
regards,
Man Zeng