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;

‎src/backend/catalog/pg_inherits.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,6 @@ find_all_inheritors(Oid parentrelId, LOCKMODE lockmode, List **numparents)
171171
*rel_numparents;
172172
ListCell *l;
173173

174-
memset(&ctl, 0, sizeof(ctl));
175174
ctl.keysize = sizeof(Oid);
176175
ctl.entrysize = sizeof(SeenRelsEntry);
177176
ctl.hcxt = CurrentMemoryContext;

‎src/backend/commands/async.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2375,7 +2375,6 @@ AddEventToPendingNotifies(Notification *n)
23752375
ListCell *l;
23762376

23772377
/* Create the hash table */
2378-
MemSet(&hash_ctl, 0, sizeof(hash_ctl));
23792378
hash_ctl.keysize = sizeof(Notification *);
23802379
hash_ctl.entrysize = sizeof(NotificationHash);
23812380
hash_ctl.hash = notification_hash;

‎src/backend/commands/prepare.c

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -406,15 +406,13 @@ InitQueryHashTable(void)
406406
{
407407
HASHCTL hash_ctl;
408408

409-
MemSet(&hash_ctl, 0, sizeof(hash_ctl));
410-
411409
hash_ctl.keysize = NAMEDATALEN;
412410
hash_ctl.entrysize = sizeof(PreparedStatement);
413411

414412
prepared_queries = hash_create("Prepared Queries",
415413
32,
416414
&hash_ctl,
417-
HASH_ELEM);
415+
HASH_ELEM | HASH_STRINGS);
418416
}
419417

420418
/*

‎src/backend/commands/sequence.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1087,7 +1087,6 @@ create_seq_hashtable(void)
10871087
{
10881088
HASHCTL ctl;
10891089

1090-
memset(&ctl, 0, sizeof(ctl));
10911090
ctl.keysize = sizeof(Oid);
10921091
ctl.entrysize = sizeof(SeqTableData);
10931092

‎src/backend/executor/execPartition.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -521,7 +521,6 @@ ExecHashSubPlanResultRelsByOid(ModifyTableState *mtstate,
521521
HTAB *htab;
522522
int i;
523523

524-
memset(&ctl, 0, sizeof(ctl));
525524
ctl.keysize = sizeof(Oid);
526525
ctl.entrysize = sizeof(SubplanResultRelHashElem);
527526
ctl.hcxt = CurrentMemoryContext;

‎src/backend/nodes/extensible.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,11 @@ RegisterExtensibleNodeEntry(HTAB **p_htable, const char *htable_label,
4747
{
4848
HASHCTL ctl;
4949

50-
memset(&ctl, 0, sizeof(HASHCTL));
5150
ctl.keysize = EXTNODENAME_MAX_LEN;
5251
ctl.entrysize = sizeof(ExtensibleNodeEntry);
5352

54-
*p_htable = hash_create(htable_label, 100, &ctl, HASH_ELEM);
53+
*p_htable = hash_create(htable_label, 100, &ctl,
54+
HASH_ELEM | HASH_STRINGS);
5555
}
5656

5757
if (strlen(extnodename) >= EXTNODENAME_MAX_LEN)

‎src/backend/optimizer/util/predtest.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1982,7 +1982,6 @@ lookup_proof_cache(Oid pred_op, Oid clause_op, bool refute_it)
19821982
/* First time through: initialize the hash table */
19831983
HASHCTL ctl;
19841984

1985-
MemSet(&ctl, 0, sizeof(ctl));
19861985
ctl.keysize = sizeof(OprProofCacheKey);
19871986
ctl.entrysize = sizeof(OprProofCacheEntry);
19881987
OprProofCacheHash = hash_create("Btree proof lookup cache", 256,

‎src/backend/optimizer/util/relnode.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,6 @@ build_join_rel_hash(PlannerInfo *root)
400400
ListCell *l;
401401

402402
/* Create the hash table */
403-
MemSet(&hash_ctl, 0, sizeof(hash_ctl));
404403
hash_ctl.keysize = sizeof(Relids);
405404
hash_ctl.entrysize = sizeof(JoinHashEntry);
406405
hash_ctl.hash = bitmap_hash;

‎src/backend/parser/parse_oper.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -999,7 +999,6 @@ find_oper_cache_entry(OprCacheKey *key)
999999
/* First time through: initialize the hash table */
10001000
HASHCTL ctl;
10011001

1002-
MemSet(&ctl, 0, sizeof(ctl));
10031002
ctl.keysize = sizeof(OprCacheKey);
10041003
ctl.entrysize = sizeof(OprCacheEntry);
10051004
OprCacheHash = hash_create("Operator lookup cache", 256,

‎src/backend/partitioning/partdesc.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -286,13 +286,13 @@ CreatePartitionDirectory(MemoryContext mcxt)
286286
PartitionDirectory pdir;
287287
HASHCTL ctl;
288288

289-
MemSet(&ctl, 0, sizeof(HASHCTL));
289+
pdir = palloc(sizeof(PartitionDirectoryData));
290+
pdir->pdir_mcxt = mcxt;
291+
290292
ctl.keysize = sizeof(Oid);
291293
ctl.entrysize = sizeof(PartitionDirectoryEntry);
292294
ctl.hcxt = mcxt;
293295

294-
pdir = palloc(sizeof(PartitionDirectoryData));
295-
pdir->pdir_mcxt = mcxt;
296296
pdir->pdir_hash = hash_create("partition directory", 256, &ctl,
297297
HASH_ELEM | HASH_BLOBS | HASH_CONTEXT);
298298

‎src/backend/postmaster/autovacuum.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2043,7 +2043,6 @@ do_autovacuum(void)
20432043
pg_class_desc = CreateTupleDescCopy(RelationGetDescr(classRel));
20442044

20452045
/* create hash table for toast <-> main relid mapping */
2046-
MemSet(&ctl, 0, sizeof(ctl));
20472046
ctl.keysize = sizeof(Oid);
20482047
ctl.entrysize = sizeof(av_relation);
20492048

‎src/backend/postmaster/checkpointer.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1161,7 +1161,6 @@ CompactCheckpointerRequestQueue(void)
11611161
skip_slot = palloc0(sizeof(bool) * CheckpointerShmem->num_requests);
11621162

11631163
/* Initialize temporary hash table */
1164-
MemSet(&ctl, 0, sizeof(ctl));
11651164
ctl.keysize = sizeof(CheckpointerRequest);
11661165
ctl.entrysize = sizeof(struct CheckpointerSlotMapping);
11671166
ctl.hcxt = CurrentMemoryContext;

‎src/backend/postmaster/pgstat.c

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1265,7 +1265,6 @@ pgstat_collect_oids(Oid catalogid, AttrNumber anum_oid)
12651265
HeapTuple tup;
12661266
Snapshot snapshot;
12671267

1268-
memset(&hash_ctl, 0, sizeof(hash_ctl));
12691268
hash_ctl.keysize = sizeof(Oid);
12701269
hash_ctl.entrysize = sizeof(Oid);
12711270
hash_ctl.hcxt = CurrentMemoryContext;
@@ -1815,7 +1814,6 @@ pgstat_init_function_usage(FunctionCallInfo fcinfo,
18151814
/* First time through - initialize function stat table */
18161815
HASHCTL hash_ctl;
18171816

1818-
memset(&hash_ctl, 0, sizeof(hash_ctl));
18191817
hash_ctl.keysize = sizeof(Oid);
18201818
hash_ctl.entrysize = sizeof(PgStat_BackendFunctionEntry);
18211819
pgStatFunctions = hash_create("Function stat entries",
@@ -1975,7 +1973,6 @@ get_tabstat_entry(Oid rel_id, bool isshared)
19751973
{
19761974
HASHCTL ctl;
19771975

1978-
memset(&ctl, 0, sizeof(ctl));
19791976
ctl.keysize = sizeof(Oid);
19801977
ctl.entrysize = sizeof(TabStatHashEntry);
19811978

@@ -4994,7 +4991,6 @@ reset_dbentry_counters(PgStat_StatDBEntry *dbentry)
49944991
dbentry->stat_reset_timestamp = GetCurrentTimestamp();
49954992
dbentry->stats_timestamp = 0;
49964993

4997-
memset(&hash_ctl, 0, sizeof(hash_ctl));
49984994
hash_ctl.keysize = sizeof(Oid);
49994995
hash_ctl.entrysize = sizeof(PgStat_StatTabEntry);
50004996
dbentry->tables = hash_create("Per-database table",
@@ -5423,7 +5419,6 @@ pgstat_read_statsfiles(Oid onlydb, bool permanent, bool deep)
54235419
/*
54245420
* Create the DB hashtable
54255421
*/
5426-
memset(&hash_ctl, 0, sizeof(hash_ctl));
54275422
hash_ctl.keysize = sizeof(Oid);
54285423
hash_ctl.entrysize = sizeof(PgStat_StatDBEntry);
54295424
hash_ctl.hcxt = pgStatLocalContext;
@@ -5608,7 +5603,6 @@ pgstat_read_statsfiles(Oid onlydb, bool permanent, bool deep)
56085603
break;
56095604
}
56105605

5611-
memset(&hash_ctl, 0, sizeof(hash_ctl));
56125606
hash_ctl.keysize = sizeof(Oid);
56135607
hash_ctl.entrysize = sizeof(PgStat_StatTabEntry);
56145608
hash_ctl.hcxt = pgStatLocalContext;

‎src/backend/replication/logical/relation.c

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,6 @@ logicalrep_relmap_init(void)
111111
ALLOCSET_DEFAULT_SIZES);
112112

113113
/* Initialize the relation hash table. */
114-
MemSet(&ctl, 0, sizeof(ctl));
115114
ctl.keysize = sizeof(LogicalRepRelId);
116115
ctl.entrysize = sizeof(LogicalRepRelMapEntry);
117116
ctl.hcxt = LogicalRepRelMapContext;
@@ -120,7 +119,6 @@ logicalrep_relmap_init(void)
120119
HASH_ELEM | HASH_BLOBS | HASH_CONTEXT);
121120

122121
/* Initialize the type hash table. */
123-
MemSet(&ctl, 0, sizeof(ctl));
124122
ctl.keysize = sizeof(Oid);
125123
ctl.entrysize = sizeof(LogicalRepTyp);
126124
ctl.hcxt = LogicalRepRelMapContext;
@@ -606,7 +604,6 @@ logicalrep_partmap_init(void)
606604
ALLOCSET_DEFAULT_SIZES);
607605

608606
/* Initialize the relation hash table. */
609-
MemSet(&ctl, 0, sizeof(ctl));
610607
ctl.keysize = sizeof(Oid); /* partition OID */
611608
ctl.entrysize = sizeof(LogicalRepPartMapEntry);
612609
ctl.hcxt = LogicalRepPartMapContext;

‎src/backend/replication/logical/reorderbuffer.c

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1619,8 +1619,6 @@ ReorderBufferBuildTupleCidHash(ReorderBuffer *rb, ReorderBufferTXN *txn)
16191619
if (!rbtxn_has_catalog_changes(txn) || dlist_is_empty(&txn->tuplecids))
16201620
return;
16211621

1622-
memset(&hash_ctl, 0, sizeof(hash_ctl));
1623-
16241622
hash_ctl.keysize = sizeof(ReorderBufferTupleCidKey);
16251623
hash_ctl.entrysize = sizeof(ReorderBufferTupleCidEnt);
16261624
hash_ctl.hcxt = rb->context;
@@ -4116,7 +4114,6 @@ ReorderBufferToastInitHash(ReorderBuffer *rb, ReorderBufferTXN *txn)
41164114

41174115
Assert(txn->toast_hash == NULL);
41184116

4119-
memset(&hash_ctl, 0, sizeof(hash_ctl));
41204117
hash_ctl.keysize = sizeof(Oid);
41214118
hash_ctl.entrysize = sizeof(ReorderBufferToastEnt);
41224119
hash_ctl.hcxt = rb->context;

‎src/backend/replication/logical/tablesync.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -372,7 +372,6 @@ process_syncing_tables_for_apply(XLogRecPtr current_lsn)
372372
{
373373
HASHCTL ctl;
374374

375-
memset(&ctl, 0, sizeof(ctl));
376375
ctl.keysize = sizeof(Oid);
377376
ctl.entrysize = sizeof(struct tablesync_start_time_mapping);
378377
last_start_times = hash_create("Logical replication table sync worker start times",

‎src/backend/replication/pgoutput/pgoutput.c

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -867,22 +867,18 @@ static void
867867
init_rel_sync_cache(MemoryContext cachectx)
868868
{
869869
HASHCTL ctl;
870-
MemoryContext old_ctxt;
871870

872871
if (RelationSyncCache != NULL)
873872
return;
874873

875874
/* Make a new hash table for the cache */
876-
MemSet(&ctl, 0, sizeof(ctl));
877875
ctl.keysize = sizeof(Oid);
878876
ctl.entrysize = sizeof(RelationSyncEntry);
879877
ctl.hcxt = cachectx;
880878

881-
old_ctxt = MemoryContextSwitchTo(cachectx);
882879
RelationSyncCache = hash_create("logical replication output relation cache",
883880
128, &ctl,
884881
HASH_ELEM | HASH_CONTEXT | HASH_BLOBS);
885-
(void) MemoryContextSwitchTo(old_ctxt);
886882

887883
Assert(RelationSyncCache != NULL);
888884

‎src/backend/storage/buffer/bufmgr.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2505,7 +2505,6 @@ InitBufferPoolAccess(void)
25052505

25062506
memset(&PrivateRefCountArray, 0, sizeof(PrivateRefCountArray));
25072507

2508-
MemSet(&hash_ctl, 0, sizeof(hash_ctl));
25092508
hash_ctl.keysize = sizeof(int32);
25102509
hash_ctl.entrysize = sizeof(PrivateRefCountEntry);
25112510

‎src/backend/storage/buffer/localbuf.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -465,7 +465,6 @@ InitLocalBuffers(void)
465465
}
466466

467467
/* Create the lookup hash table */
468-
MemSet(&info, 0, sizeof(info));
469468
info.keysize = sizeof(BufferTag);
470469
info.entrysize = sizeof(LocalBufferLookupEnt);
471470

0 commit comments

Comments
 (0)