cleanup patches for dshash

From af203e6e80f7a2843890e51fd3e3641aef8bd901 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Fri, 19 Jan 2024 15:23:35 -0600
Subject: [PATCH v2 1/2] Use NULL instead of 0 for arg parameter in
 dshash_create().

---
 src/backend/replication/logical/launcher.c | 2 +-
 src/backend/utils/activity/pgstat_shmem.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/backend/replication/logical/launcher.c b/src/backend/replication/logical/launcher.c
index 122db0bb13..ee66c292bd 100644
--- a/src/backend/replication/logical/launcher.c
+++ b/src/backend/replication/logical/launcher.c
@@ -1013,7 +1013,7 @@ logicalrep_launcher_attach_dshmem(void)
 		last_start_times_dsa = dsa_create(LWTRANCHE_LAUNCHER_DSA);
 		dsa_pin(last_start_times_dsa);
 		dsa_pin_mapping(last_start_times_dsa);
-		last_start_times = dshash_create(last_start_times_dsa, &dsh_params, 0);
+		last_start_times = dshash_create(last_start_times_dsa, &dsh_params, NULL);
 
 		/* Store handles in shared memory for other backends to use. */
 		LogicalRepCtx->last_start_dsa = dsa_get_handle(last_start_times_dsa);
diff --git a/src/backend/utils/activity/pgstat_shmem.c b/src/backend/utils/activity/pgstat_shmem.c
index 3ce48e88a3..71410ddd3f 100644
--- a/src/backend/utils/activity/pgstat_shmem.c
+++ b/src/backend/utils/activity/pgstat_shmem.c
@@ -180,7 +180,7 @@ StatsShmemInit(void)
 		 * With the limit in place, create the dshash table. XXX: It'd be nice
 		 * if there were dshash_create_in_place().
 		 */
-		dsh = dshash_create(dsa, &dsh_params, 0);
+		dsh = dshash_create(dsa, &dsh_params, NULL);
 		ctl->hash_handle = dshash_get_hash_table_handle(dsh);
 
 		/* lift limit set above */
-- 
2.25.1

From 1d328f0be681da7e43574350a1d3c8da8ae8ddcc Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Fri, 19 Jan 2024 15:38:34 -0600
Subject: [PATCH v2 2/2] Add hash/compare functions for dshash tables with
 string keys.

---
 src/backend/lib/dshash.c                   | 58 +++++++++++++++++++++-
 src/backend/replication/logical/launcher.c |  1 +
 src/backend/storage/ipc/dsm_registry.c     |  9 ++--
 src/backend/utils/activity/pgstat_shmem.c  |  1 +
 src/backend/utils/cache/typcache.c         |  2 +
 src/include/lib/dshash.h                   | 19 ++++++-
 6 files changed, 83 insertions(+), 7 deletions(-)

diff --git a/src/backend/lib/dshash.c b/src/backend/lib/dshash.c
index b0bc0abda0..cc49b4ca51 100644
--- a/src/backend/lib/dshash.c
+++ b/src/backend/lib/dshash.c
@@ -188,6 +188,8 @@ static bool delete_item_from_bucket(dshash_table *hash_table,
 static inline dshash_hash hash_key(dshash_table *hash_table, const void *key);
 static inline bool equal_keys(dshash_table *hash_table,
 							  const void *a, const void *b);
+static inline void copy_key(dshash_table *hash_table, void *dest,
+							const void *src);
 
 #define PARTITION_LOCK(hash_table, i)			\
 	(&(hash_table)->control->partitions[(i)].lock)
@@ -583,6 +585,49 @@ dshash_memhash(const void *v, size_t size, void *arg)
 	return tag_hash(v, size);
 }
 
+/*
+ * A copy function that forwards to memcpy.
+ */
+void
+dshash_memcpy(void *dest, const void *src, size_t size, void *arg)
+{
+	(void) memcpy(dest, src, size);
+}
+
+/*
+ * A compare function that forwards to strcmp.
+ */
+int
+dshash_strcmp(const void *a, const void *b, size_t size, void *arg)
+{
+	Assert(strlen((const char *) a) < size);
+	Assert(strlen((const char *) b) < size);
+
+	return strcmp((const char *) a, (const char *) b);
+}
+
+/*
+ * A hash function that forwards to string_hash.
+ */
+dshash_hash
+dshash_strhash(const void *v, size_t size, void *arg)
+{
+	Assert(strlen((const char *) v) < size);
+
+	return string_hash((const char *) v, size);
+}
+
+/*
+ * A copy function that forwards to strcpy.
+ */
+void
+dshash_strcpy(void *dest, const void *src, size_t size, void *arg)
+{
+	Assert(strlen((const char *) src) < size);
+
+	(void) strcpy((char *) dest, (const char *) src);
+}
+
 /*
  * Sequentially scan through dshash table and return all the elements one by
  * one, return NULL when all elements have been returned.
@@ -949,7 +994,7 @@ insert_into_bucket(dshash_table *hash_table,
 								hash_table->params.entry_size +
 								MAXALIGN(sizeof(dshash_table_item)));
 	item = dsa_get_address(hash_table->area, item_pointer);
-	memcpy(ENTRY_FROM_ITEM(item), key, hash_table->params.key_size);
+	copy_key(hash_table, ENTRY_FROM_ITEM(item), key);
 	insert_item_into_bucket(hash_table, item_pointer, item, bucket);
 	return item;
 }
@@ -1032,3 +1077,14 @@ equal_keys(dshash_table *hash_table, const void *a, const void *b)
 											   hash_table->params.key_size,
 											   hash_table->arg) == 0;
 }
+
+/*
+ * Copy a key.
+ */
+static inline void
+copy_key(dshash_table *hash_table, void *dest, const void *src)
+{
+	hash_table->params.copy_function(dest, src,
+									 hash_table->params.key_size,
+									 hash_table->arg);
+}
diff --git a/src/backend/replication/logical/launcher.c b/src/backend/replication/logical/launcher.c
index ee66c292bd..487f141a59 100644
--- a/src/backend/replication/logical/launcher.c
+++ b/src/backend/replication/logical/launcher.c
@@ -88,6 +88,7 @@ static const dshash_parameters dsh_params = {
 	sizeof(LauncherLastStartTimesEntry),
 	dshash_memcmp,
 	dshash_memhash,
+	dshash_memcpy,
 	LWTRANCHE_LAUNCHER_HASH
 };
 
diff --git a/src/backend/storage/ipc/dsm_registry.c b/src/backend/storage/ipc/dsm_registry.c
index ac11f51375..e49edf52b4 100644
--- a/src/backend/storage/ipc/dsm_registry.c
+++ b/src/backend/storage/ipc/dsm_registry.c
@@ -50,8 +50,9 @@ typedef struct DSMRegistryEntry
 static const dshash_parameters dsh_params = {
 	offsetof(DSMRegistryEntry, handle),
 	sizeof(DSMRegistryEntry),
-	dshash_memcmp,
-	dshash_memhash,
+	dshash_strcmp,
+	dshash_strhash,
+	dshash_strcpy,
 	LWTRANCHE_DSM_REGISTRY_HASH
 };
 
@@ -132,7 +133,6 @@ GetNamedDSMSegment(const char *name, size_t size,
 {
 	DSMRegistryEntry *entry;
 	MemoryContext oldcontext;
-	char		name_padded[offsetof(DSMRegistryEntry, handle)] = {0};
 	void	   *ret;
 
 	Assert(found);
@@ -155,8 +155,7 @@ GetNamedDSMSegment(const char *name, size_t size,
 	/* Connect to the registry. */
 	init_dsm_registry();
 
-	strcpy(name_padded, name);
-	entry = dshash_find_or_insert(dsm_registry_table, name_padded, found);
+	entry = dshash_find_or_insert(dsm_registry_table, name, found);
 	if (!(*found))
 	{
 		/* Initialize the segment. */
diff --git a/src/backend/utils/activity/pgstat_shmem.c b/src/backend/utils/activity/pgstat_shmem.c
index 71410ddd3f..91591da395 100644
--- a/src/backend/utils/activity/pgstat_shmem.c
+++ b/src/backend/utils/activity/pgstat_shmem.c
@@ -64,6 +64,7 @@ static const dshash_parameters dsh_params = {
 	sizeof(PgStatShared_HashEntry),
 	pgstat_cmp_hash_key,
 	pgstat_hash_hash_key,
+	dshash_memcpy,
 	LWTRANCHE_PGSTATS_HASH
 };
 
diff --git a/src/backend/utils/cache/typcache.c b/src/backend/utils/cache/typcache.c
index 84fc83cb11..bd7bd798f0 100644
--- a/src/backend/utils/cache/typcache.c
+++ b/src/backend/utils/cache/typcache.c
@@ -258,6 +258,7 @@ static const dshash_parameters srtr_record_table_params = {
 	sizeof(SharedRecordTableEntry),
 	shared_record_table_compare,
 	shared_record_table_hash,
+	dshash_memcpy,
 	LWTRANCHE_PER_SESSION_RECORD_TYPE
 };
 
@@ -267,6 +268,7 @@ static const dshash_parameters srtr_typmod_table_params = {
 	sizeof(SharedTypmodTableEntry),
 	dshash_memcmp,
 	dshash_memhash,
+	dshash_memcpy,
 	LWTRANCHE_PER_SESSION_RECORD_TYPMOD
 };
 
diff --git a/src/include/lib/dshash.h b/src/include/lib/dshash.h
index 91f70ac2b5..2ff1ba6c24 100644
--- a/src/include/lib/dshash.h
+++ b/src/include/lib/dshash.h
@@ -37,6 +37,10 @@ typedef int (*dshash_compare_function) (const void *a, const void *b,
 typedef dshash_hash (*dshash_hash_function) (const void *v, size_t size,
 											 void *arg);
 
+/* A function type for copying keys. */
+typedef void (*dshash_copy_function) (void *dest, const void *src, size_t size,
+									  void *arg);
+
 /*
  * The set of parameters needed to create or attach to a hash table.  The
  * members tranche_id and tranche_name do not need to be initialized when
@@ -55,6 +59,7 @@ typedef struct dshash_parameters
 	size_t		entry_size;		/* Total size of entry */
 	dshash_compare_function compare_function;	/* Compare function */
 	dshash_hash_function hash_function; /* Hash function */
+	dshash_copy_function copy_function; /* Copy function */
 	int			tranche_id;		/* The tranche ID to use for locks */
 } dshash_parameters;
 
@@ -105,9 +110,21 @@ extern void *dshash_seq_next(dshash_seq_status *status);
 extern void dshash_seq_term(dshash_seq_status *status);
 extern void dshash_delete_current(dshash_seq_status *status);
 
-/* Convenience hash and compare functions wrapping memcmp and tag_hash. */
+/*
+ * Convenience hash, compare, and copy functions wrapping memcmp, tag_hash, and
+ * memcpy.
+ */
 extern int	dshash_memcmp(const void *a, const void *b, size_t size, void *arg);
 extern dshash_hash dshash_memhash(const void *v, size_t size, void *arg);
+extern void dshash_memcpy(void *dest, const void *src, size_t size, void *arg);
+
+/*
+ * Convenience hash, compare, and copy functions wrapping strcmp, string_hash,
+ * and strcpy.
+ */
+extern int	dshash_strcmp(const void *a, const void *b, size_t size, void *arg);
+extern dshash_hash dshash_strhash(const void *v, size_t size, void *arg);
+extern void dshash_strcpy(void *dest, const void *src, size_t size, void *arg);
 
 /* Debugging support. */
 extern void dshash_dump(dshash_table *hash_table);
-- 
2.25.1

From 298b98552f7e5586a268de9aeaec533631a9f608 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Mon, 26 Feb 2024 22:33:41 -0600
Subject: [PATCH 1/1] Fix comments for the dshash_parameters struct.

I recently added a copy_function member to the dshash_parameters
struct, but I forgot to update a couple of comments that refer to
the function pointer members of this struct.  While fixing that, I
noticed that one comment refers to a tranche_name member that was
removed during development of dshash tables.  The same comment also
refers to non-arg variants of the function pointer members, but
those were removed shortly after dshash table support was first
committed.

Oversights in commits 8c0d7bafad, d7694fc148, and 42a1de3013.
---
 src/backend/lib/dshash.c |  2 +-
 src/include/lib/dshash.h | 14 ++++++--------
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/src/backend/lib/dshash.c b/src/backend/lib/dshash.c
index cc49b4ca51..ab30f29eee 100644
--- a/src/backend/lib/dshash.c
+++ b/src/backend/lib/dshash.c
@@ -202,7 +202,7 @@ static inline void copy_key(dshash_table *hash_table, void *dest,
  * Create a new hash table backed by the given dynamic shared area, with the
  * given parameters.  The returned object is allocated in backend-local memory
  * using the current MemoryContext.  'arg' will be passed through to the
- * compare and hash functions.
+ * compare, hash, and copy functions.
  */
 dshash_table *
 dshash_create(dsa_area *area, const dshash_parameters *params, void *arg)
diff --git a/src/include/lib/dshash.h b/src/include/lib/dshash.h
index 2ff1ba6c24..e26f3ef97f 100644
--- a/src/include/lib/dshash.h
+++ b/src/include/lib/dshash.h
@@ -43,15 +43,13 @@ typedef void (*dshash_copy_function) (void *dest, const void *src, size_t size,
 
 /*
  * The set of parameters needed to create or attach to a hash table.  The
- * members tranche_id and tranche_name do not need to be initialized when
- * attaching to an existing hash table.
+ * tranche_id does not need to be initialized when attaching to an existing
+ * hash table.
  *
- * Compare and hash functions must be supplied even when attaching, because we
- * can't safely share function pointers between backends in general.  Either
- * the arg variants or the non-arg variants should be supplied; the other
- * function pointers should be NULL.  If the arg variants are supplied then the
- * user data pointer supplied to the create and attach functions will be
- * passed to the hash and compare functions.
+ * Compare, hash, and copy functions must be supplied even when attaching,
+ * because we can't safely share function pointers between backends in general.
+ * The user data pointer supplied to the create and attach functions will be
+ * passed to these functions.
  */
 typedef struct dshash_parameters
 {
-- 
2.25.1