Skip to content

Commit b3817f5

Browse files
tglsfdctglsfdc
committed
Improve hash_create()'s API for some added robustness.
Invent a new flag bit HASH_STRINGS to specify C-string hashing, which was formerly the default; and add assertions insisting that exactly one of the bits HASH_STRINGS, HASH_BLOBS, and HASH_FUNCTION be set. This is in hopes of preventing recurrences of the type of oversight fixed in commit a1b8aa1 (i.e., mistakenly omitting HASH_BLOBS). Also, when HASH_STRINGS is specified, insist that the keysize be more than 8 bytes. This is a heuristic, but it should catch accidental use of HASH_STRINGS for integer or pointer keys. (Nearly all existing use-cases set the keysize to NAMEDATALEN or more, so there's little reason to think this restriction should be problematic.) Tweak hash_create() to insist that the HASH_ELEM flag be set, and remove the defaults it had for keysize and entrysize. Since those defaults were undocumented and basically useless, no callers omitted HASH_ELEM anyway. Also, remove memset's zeroing the HASHCTL parameter struct from those callers that had one. This has never been really necessary, and while it wasn't a bad coding convention it was confusing that some callers did it and some did not. We might as well save a few cycles by standardizing on "not". Also improve the documentation for hash_create(). In passing, improve reinit.c's usage of a hash table by storing the key as a binary Oid rather than a string; and, since that's a temporary hash table, allocate it in CurrentMemoryContext for neatness. Discussion: https://postgr.es/m/590625.1607878171@sss.pgh.pa.us
1 parent a58db3a commit b3817f5

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

63 files changed

+112
-158
lines changed

‎contrib/dblink/dblink.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2607,7 +2607,8 @@ createConnHash(void)
26072607
ctl.keysize = NAMEDATALEN;
26082608
ctl.entrysize = sizeof(remoteConnHashEnt);
26092609

2610-
return hash_create("Remote Con hash", NUMCONN, &ctl, HASH_ELEM);
2610+
return hash_create("Remote Con hash", NUMCONN, &ctl,
2611+
HASH_ELEM | HASH_STRINGS);
26112612
}
26122613

26132614
static void

‎contrib/pg_stat_statements/pg_stat_statements.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -567,7 +567,6 @@ pgss_shmem_startup(void)
567567
pgss->stats.dealloc = 0;
568568
}
569569

570-
memset(&info, 0, sizeof(info));
571570
info.keysize = sizeof(pgssHashKey);
572571
info.entrysize = sizeof(pgssEntry);
573572
pgss_hash = ShmemInitHash("pg_stat_statements hash",

‎contrib/postgres_fdw/connection.c

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -119,14 +119,11 @@ GetConnection(UserMapping *user, bool will_prep_stmt)
119119
{
120120
HASHCTL ctl;
121121

122-
MemSet(&ctl, 0, sizeof(ctl));
123122
ctl.keysize = sizeof(ConnCacheKey);
124123
ctl.entrysize = sizeof(ConnCacheEntry);
125-
/* allocate ConnectionHash in the cache context */
126-
ctl.hcxt = CacheMemoryContext;
127124
ConnectionHash = hash_create("postgres_fdw connections", 8,
128125
&ctl,
129-
HASH_ELEM | HASH_BLOBS | HASH_CONTEXT);
126+
HASH_ELEM | HASH_BLOBS);
130127

131128
/*
132129
* Register some callback functions that manage connection cleanup.

‎contrib/postgres_fdw/shippable.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,6 @@ InitializeShippableCache(void)
9393
HASHCTL ctl;
9494

9595
/* Create the hash table. */
96-
MemSet(&ctl, 0, sizeof(ctl));
9796
ctl.keysize = sizeof(ShippableCacheKey);
9897
ctl.entrysize = sizeof(ShippableCacheEntry);
9998
ShippableCacheHash =

‎contrib/tablefunc/tablefunc.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -714,7 +714,6 @@ load_categories_hash(char *cats_sql, MemoryContext per_query_ctx)
714714
MemoryContext SPIcontext;
715715

716716
/* initialize the category hash table */
717-
MemSet(&ctl, 0, sizeof(ctl));
718717
ctl.keysize = MAX_CATNAME_LEN;
719718
ctl.entrysize = sizeof(crosstab_HashEnt);
720719
ctl.hcxt = per_query_ctx;
@@ -726,7 +725,7 @@ load_categories_hash(char *cats_sql, MemoryContext per_query_ctx)
726725
crosstab_hash = hash_create("crosstab hash",
727726
INIT_CATS,
728727
&ctl,
729-
HASH_ELEM | HASH_CONTEXT);
728+
HASH_ELEM | HASH_STRINGS | HASH_CONTEXT);
730729

731730
/* Connect to SPI manager */
732731
if ((ret = SPI_connect()) < 0)

‎src/backend/access/gist/gistbuildbuffers.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@ gistInitBuildBuffers(int pagesPerBuffer, int levelStep, int maxLevel)
7676
* nodeBuffersTab hash is association between index blocks and it's
7777
* buffers.
7878
*/
79-
memset(&hashCtl, 0, sizeof(hashCtl));
8079
hashCtl.keysize = sizeof(BlockNumber);
8180
hashCtl.entrysize = sizeof(GISTNodeBuffer);
8281
hashCtl.hcxt = CurrentMemoryContext;

‎src/backend/access/hash/hashpage.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1363,7 +1363,6 @@ _hash_finish_split(Relation rel, Buffer metabuf, Buffer obuf, Bucket obucket,
13631363
bool found;
13641364

13651365
/* Initialize hash tables used to track TIDs */
1366-
memset(&hash_ctl, 0, sizeof(hash_ctl));
13671366
hash_ctl.keysize = sizeof(ItemPointerData);
13681367
hash_ctl.entrysize = sizeof(ItemPointerData);
13691368
hash_ctl.hcxt = CurrentMemoryContext;

‎src/backend/access/heap/rewriteheap.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,6 @@ begin_heap_rewrite(Relation old_heap, Relation new_heap, TransactionId oldest_xm
266266
state->rs_cxt = rw_cxt;
267267

268268
/* Initialize hash tables used to track update chains */
269-
memset(&hash_ctl, 0, sizeof(hash_ctl));
270269
hash_ctl.keysize = sizeof(TidHashKey);
271270
hash_ctl.entrysize = sizeof(UnresolvedTupData);
272271
hash_ctl.hcxt = state->rs_cxt;
@@ -824,7 +823,6 @@ logical_begin_heap_rewrite(RewriteState state)
824823
state->rs_begin_lsn = GetXLogInsertRecPtr();
825824
state->rs_num_rewrite_mappings = 0;
826825

827-
memset(&hash_ctl, 0, sizeof(hash_ctl));
828826
hash_ctl.keysize = sizeof(TransactionId);
829827
hash_ctl.entrysize = sizeof(RewriteMappingFile);
830828
hash_ctl.hcxt = state->rs_cxt;

‎src/backend/access/transam/xlogutils.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,6 @@ log_invalid_page(RelFileNode node, ForkNumber forkno, BlockNumber blkno,
113113
/* create hash table when first needed */
114114
HASHCTL ctl;
115115

116-
memset(&ctl, 0, sizeof(ctl));
117116
ctl.keysize = sizeof(xl_invalid_page_key);
118117
ctl.entrysize = sizeof(xl_invalid_page);
119118

‎src/backend/catalog/pg_enum.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,6 @@ init_enum_blacklist(void)
188188
{
189189
HASHCTL hash_ctl;
190190

191-
memset(&hash_ctl, 0, sizeof(hash_ctl));
192191
hash_ctl.keysize = sizeof(Oid);
193192
hash_ctl.entrysize = sizeof(Oid);
194193
hash_ctl.hcxt = TopTransactionContext;

0 commit comments

Comments
 (0)