[PATCH] Remove stale comment and dead store in BuildTupleHashTable
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:t253361psql -h localhost -U postgresBuilt from patchset v1 (message #1), August 25, 2026 at 01:48 PM.
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 t253361_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 t253361_1 && git checkout t253361_1Patchset v1 (message #1) is on t253361_1
Hi,
While reading the aggregation-related executor code, I noticed a comment
that no longer matches what the code does.
In BuildTupleHashTable() (src/backend/executor/execGrouping.c) we have:
hashtable->tableslot = NULL; /* will be made on first lookup */
The slot is no longer made on first lookup. Commit bf6c614a2f2 removed
the lazy creation from LookupTupleHashEntry() and made
BuildTupleHashTable() create the slot unconditionally, further down in
the same function. Only the comment and its assignment were left behind.
The attached patch removes both. Nothing reads tableslot before
slot is actually created.
Regards,
Tatsuya Kawata
On 8/9/26 7:22 PM, Tatsuya Kawata wrote:
Hi,
While reading the aggregation-related executor code, I noticed a comment
that no longer matches what the code does.In BuildTupleHashTable() (src/backend/executor/execGrouping.c) we have:
hashtable->tableslot = NULL; /* will be made on first lookup */
This initialization was initially introduced in the patch f97aebd1.
The hashtable->tableslot is initialized in the LookupTupleHashEntry
function.
The slot is no longer made on first lookup. Commit bf6c614a2f2 removed
the lazy creation from LookupTupleHashEntry() and made
BuildTupleHashTable() create the slot unconditionally, further down in
the same function. Only the comment and its assignment were left behind.
It also involves patch 773aec7a and its revert patch 2a41507d
Initialization is no longer necessary.
--
Quan Zongliang
On 8/11/26 10:01 AM, Quan Zongliang wrote:
On 8/9/26 7:22 PM, Tatsuya Kawata wrote:
Hi,
While reading the aggregation-related executor code, I noticed a comment
that no longer matches what the code does.In BuildTupleHashTable() (src/backend/executor/execGrouping.c) we have:
hashtable->tableslot = NULL; /* will be made on first lookup */
This initialization was initially introduced in the patch f97aebd1.
The hashtable->tableslot is initialized in the LookupTupleHashEntry
function.The slot is no longer made on first lookup. Commit bf6c614a2f2 removed
the lazy creation from LookupTupleHashEntry() and made
BuildTupleHashTable() create the slot unconditionally, further down in
the same function. Only the comment and its assignment were left behind.It also involves patch 773aec7a and its revert patch 2a41507d
Initialization is no longer necessary.
This is a very clear and straightforward code refactoring. There won't
be any objections. Change to "Ready for Committer".
--
Quan Zongliang