Hi, 0001: hash_freeze() has not been used since 5dfc198146 (Oct 2016). I suggest removing it.
0002: hash_destroy() is the only caller of hash_stats(). I suggest making the function static and place it under `#ifdef HASH_STATISTICS`. Thoughts? -- Best regards, Aleksander Alekseev
From 8e7c526a975023ab72141d513bbcc8e352815c53 Mon Sep 17 00:00:00 2001 From: Aleksander Alekseev <[email protected]> Date: Thu, 29 Oct 2026 13:31:32 +0300 Subject: [PATCH v1 2/2] Make hash_stats() static The function was declared in hsearch.h, but it is not part of the dynahash API. Make it static and compile only under HASH_STATISTICS. Author: Aleksander Alekseev <[email protected]> Reviewed-by: TODO FIXME Discussion: TODO FIXME --- src/backend/utils/hash/dynahash.c | 15 ++++++++++----- src/include/utils/hsearch.h | 1 - 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/backend/utils/hash/dynahash.c b/src/backend/utils/hash/dynahash.c index 47d3cacc532..4e57009cfbb 100644 --- a/src/backend/utils/hash/dynahash.c +++ b/src/backend/utils/hash/dynahash.c @@ -80,8 +80,8 @@ * are not implemented; otherwise functionality is identical. * * Compilation controls: - * HASH_STATISTICS causes some usage statistics to be maintained, which can be - * logged by calling hash_stats(). + * HASH_STATISTICS causes some usage statistics to be maintained, which are + * logged when the hashtable is destroyed. * * Problems & fixes to [email protected]. WARNING: relies on pre-processor * concatenation property, in probably unnecessary code 'optimization'. @@ -285,6 +285,9 @@ static int next_pow2_int(int64 num); static void register_seq_scan(HTAB *hashp); static void deregister_seq_scan(HTAB *hashp); static bool has_seq_scans(HTAB *hashp); +#ifdef HASH_STATISTICS +static void hash_stats(const char *caller, HTAB *hashp); +#endif /* @@ -803,7 +806,9 @@ hash_destroy(HTAB *hashp) /* so this hashtable must have its own context */ Assert(hashp->hcxt != NULL); +#ifdef HASH_STATISTICS hash_stats(__func__, hashp); +#endif /* * Free everything by destroying the hash table's memory context. @@ -812,10 +817,10 @@ hash_destroy(HTAB *hashp) } } -void +#ifdef HASH_STATISTICS +static void hash_stats(const char *caller, HTAB *hashp) { -#ifdef HASH_STATISTICS HASHHDR *hctl = hashp->hctl; elog(DEBUG4, @@ -823,8 +828,8 @@ hash_stats(const char *caller, HTAB *hashp) caller != NULL ? caller : "(unknown)", hashp->tabname, hctl->accesses, hctl->collisions, hctl->expansions, hash_get_num_entries(hashp), hctl->keysize, hctl->max_bucket, hctl->nsegs); -#endif } +#endif /*******************************SEARCH ROUTINES *****************************/ diff --git a/src/include/utils/hsearch.h b/src/include/utils/hsearch.h index c238466c3eb..59180719b9d 100644 --- a/src/include/utils/hsearch.h +++ b/src/include/utils/hsearch.h @@ -127,7 +127,6 @@ typedef struct extern HTAB *hash_create(const char *tabname, int64 nelem, const HASHCTL *info, int flags); extern void hash_destroy(HTAB *hashp); -extern void hash_stats(const char *caller, HTAB *hashp); extern void *hash_search(HTAB *hashp, const void *keyPtr, HASHACTION action, bool *foundPtr); extern uint32 get_hash_value(HTAB *hashp, const void *keyPtr); -- 2.43.0
From 73213c250a6beed70a55ad20426476ef56218672 Mon Sep 17 00:00:00 2001 From: Aleksander Alekseev <[email protected]> Date: Thu, 29 Oct 2026 13:26:11 +0300 Subject: [PATCH v1 1/2] Remove hash_freeze() hash_freeze() was added by a2e923a652. Its only caller was execGrouping.c, which stopped using dynahash in 5dfc198146. Nothing in the tree has called the function since, so remove it. Author: Aleksander Alekseev <[email protected]> Reviewed-by: TODO FIXME Discussion: TODO FIXME --- src/backend/utils/hash/dynahash.c | 55 +++---------------------------- src/include/utils/hsearch.h | 1 - 2 files changed, 5 insertions(+), 51 deletions(-) diff --git a/src/backend/utils/hash/dynahash.c b/src/backend/utils/hash/dynahash.c index 0db984a26c0..47d3cacc532 100644 --- a/src/backend/utils/hash/dynahash.c +++ b/src/backend/utils/hash/dynahash.c @@ -234,9 +234,6 @@ struct HTAB char *tabname; /* table name (for error messages) */ bool isshared; /* true if table is in shared memory */ - /* freezing a shared table isn't allowed, so we can keep state here */ - bool frozen; /* true = no more inserts allowed */ - /* We keep local copies of these fixed values to reduce contention */ Size keysize; /* hash key length in bytes */ @@ -534,8 +531,6 @@ hash_create(const char *tabname, int64 nelem, const HASHCTL *info, int flags) (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("out of memory"))); - hashp->frozen = false; - hdefault(hashp); hctl = hashp->hctl; @@ -928,11 +923,11 @@ hash_search_with_hash_value(HTAB *hashp, if (action == HASH_ENTER || action == HASH_ENTER_NULL) { /* - * Can't split if running in partitioned mode, nor if frozen, nor if - * table is the subject of any active hash_seq_search scans. + * Can't split if running in partitioned mode, nor if table is the + * subject of any active hash_seq_search scans. */ if (hctl->freeList[0].nentries > (int64) hctl->max_bucket && - !hctl->isfixed && !IS_PARTITIONED(hctl) && !hashp->frozen && + !hctl->isfixed && !IS_PARTITIONED(hctl) && !has_seq_scans(hashp)) (void) expand_table(hashp); } @@ -1010,11 +1005,6 @@ hash_search_with_hash_value(HTAB *hashp, if (currBucket != NULL) return ELEMENTKEY(currBucket); - /* disallow inserts if frozen */ - if (hashp->frozen) - elog(ERROR, "cannot insert into frozen hashtable \"%s\"", - hashp->tabname); - currBucket = get_hash_entry(hashp, freelist_idx); if (currBucket == NULL) { @@ -1096,11 +1086,6 @@ hash_update_hash_key(HTAB *hashp, hctl->accesses++; #endif - /* disallow updates if frozen */ - if (hashp->frozen) - elog(ERROR, "cannot update in frozen hashtable \"%s\"", - hashp->tabname); - /* * Lookup the existing element using its saved hash value. We need to do * this to be able to unlink it from its hash chain, but as a side benefit @@ -1306,10 +1291,6 @@ hash_get_num_entries(HTAB *hashp) * if elements are added to the table while the scan is in progress, it is * unspecified whether they will be visited by the scan or not. * - * NOTE: it is possible to use hash_seq_init/hash_seq_search without any - * worry about hash_seq_term cleanup, if the hashtable is first locked against - * further insertions by calling hash_freeze. - * * NOTE: to use this with a partitioned hashtable, caller had better hold * at least shared lock on all partitions of the table throughout the scan! * We can cope with insertions or deletions by our own backend, but *not* @@ -1322,8 +1303,7 @@ hash_seq_init(HASH_SEQ_STATUS *status, HTAB *hashp) status->curBucket = 0; status->curEntry = NULL; status->hasHashvalue = false; - if (!hashp->frozen) - register_seq_scan(hashp); + register_seq_scan(hashp); } /* @@ -1445,32 +1425,7 @@ hash_seq_search(HASH_SEQ_STATUS *status) void hash_seq_term(HASH_SEQ_STATUS *status) { - if (!status->hashp->frozen) - deregister_seq_scan(status->hashp); -} - -/* - * hash_freeze - * Freeze a hashtable against future insertions (deletions are - * still allowed) - * - * The reason for doing this is that by preventing any more bucket splits, - * we no longer need to worry about registering hash_seq_search scans, - * and thus caller need not be careful about ensuring hash_seq_term gets - * called at the right times. - * - * Multiple calls to hash_freeze() are allowed, but you can't freeze a table - * with active scans (since hash_seq_term would then do the wrong thing). - */ -void -hash_freeze(HTAB *hashp) -{ - if (hashp->isshared) - elog(ERROR, "cannot freeze shared hashtable \"%s\"", hashp->tabname); - if (!hashp->frozen && has_seq_scans(hashp)) - elog(ERROR, "cannot freeze hashtable \"%s\" because it has active scans", - hashp->tabname); - hashp->frozen = true; + deregister_seq_scan(status->hashp); } diff --git a/src/include/utils/hsearch.h b/src/include/utils/hsearch.h index 896a22a2142..c238466c3eb 100644 --- a/src/include/utils/hsearch.h +++ b/src/include/utils/hsearch.h @@ -143,7 +143,6 @@ extern void hash_seq_init_with_hash_value(HASH_SEQ_STATUS *status, uint32 hashvalue); extern void *hash_seq_search(HASH_SEQ_STATUS *status); extern void hash_seq_term(HASH_SEQ_STATUS *status); -extern void hash_freeze(HTAB *hashp); extern Size hash_estimate_size(int64 num_entries, Size entrysize); extern void AtEOXact_HashTables(bool isCommit); extern void AtEOSubXact_HashTables(bool isCommit, int nestDepth); -- 2.43.0
