[patch] Adding an assertion to report too long hash table name
Hi,
The max size for the shmem hash table name is SHMEM_INDEX_KEYSIZE - 1.
but when the caller uses a longer hash table name, it doesn't report any error, instead
it just uses the first SHMEM_INDEX_KEYSIZE -1 chars as the hash table name.
I created some shmem hash tables with the same prefix which was longer than
SHMEM_INDEX_KEYSIZE - 1, and the size of those hash tables were the same,
then only one hash table was created. But I thought those hash tables were created
successfully.
I know this is a corner case, but it's difficult to figure it out when run into it. So I add
an assertion to prevent it.
Thanks,
Xiaoran
Attachments:
Adding-an-assertion-to-report-too-long-hash-table-name.patchapplication/octet-stream; name=Adding-an-assertion-to-report-too-long-hash-table-name.patchDownload
From 67bdc0edb927dfc45f169c452eaa72a8a2f5f687 Mon Sep 17 00:00:00 2001
From: Xiaoran Wang <wxiaoran@vmware.com>
Date: Mon, 26 Sep 2022 12:56:33 +0800
Subject: [PATCH] Adding an assertion to report too long hash table name
The max size for shared memory hash table name is SHMEM_INDEX_KEYSIZE - 1
(shared hash table name is stored and indexed by ShmemIndex hash table,
the key size of it is SHMEM_INDEX_KEYSIZE), but when a caller uses a
longer hash table name, it doesn't report any error, instead it just
uses the first SHMEM_INDEX_KEYSIZE -1 chars as the hash table name.
When some hash tables' names have the same prefix which is longer than
(SHMEM_INDEX_KEYSIZE - 1), and they have the same size, then those hash
tables will be created as the same hash table. So add the assertion
to prevent it.
---
src/backend/storage/ipc/shmem.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/backend/storage/ipc/shmem.c b/src/backend/storage/ipc/shmem.c
index c1279960cd..c32e4581f7 100644
--- a/src/backend/storage/ipc/shmem.c
+++ b/src/backend/storage/ipc/shmem.c
@@ -433,6 +433,7 @@ ShmemInitStruct(const char *name, Size size, bool *foundPtr)
return structPtr;
}
+ Assert(strlen(name) < SHMEM_INDEX_KEYSIZE);
/* look it up in the shmem index */
result = (ShmemIndexEnt *)
hash_search(ShmemIndex, name, HASH_ENTER_NULL, foundPtr);
--
2.32.1 (Apple Git-133)
LGTM
+1
On Thu, Sep 29, 2022 at 9:38 AM Xiaoran Wang <wxiaoran@vmware.com> wrote:
Hi,
The max size for the shmem hash table name is SHMEM_INDEX_KEYSIZE - 1.
but when the caller uses a longer hash table name, it doesn't report any error, instead
it just uses the first SHMEM_INDEX_KEYSIZE -1 chars as the hash table name.I created some shmem hash tables with the same prefix which was longer than
SHMEM_INDEX_KEYSIZE - 1, and the size of those hash tables were the same,
then only one hash table was created. But I thought those hash tables were created
successfully.I know this is a corner case, but it's difficult to figure it out when run into it. So I add
an assertion to prevent it.Thanks,
Xiaoran
--
Regards
Junwang Zhao
Hi,
On Sep 29, 2022, 09:38 +0800, Xiaoran Wang <wxiaoran@vmware.com>, wrote:
Hi,
The max size for the shmem hash table name is SHMEM_INDEX_KEYSIZE - 1. but when the caller uses a longer hash table name, it doesn't report any error, insteadit just uses the first SHMEM_INDEX_KEYSIZE -1 chars as the hash table name.
I created some shmem hash tables with the same prefix which was longer thanSHMEM_INDEX_KEYSIZE - 1, and the size of those hash tables were the same,then only one hash table was created. But I thought those hash tables were createdsuccessfully.
I know this is a corner case, but it's difficult to figure it out when run into it. So I addan assertion to prevent it.Thanks,Xiaoran
Seems Postgres doesn’t have a case that strlen(name) >= SHMEM_INDEX_KEYSIZE(48).
The max length of name I found is 29:
```
ShmemInitHash("Shared Buffer Lookup Table”
```
But it will help for other Databases built on Postgres or later Postgres in case of forgetting to update SHMEM_INDEX_KEYSIZE
when new shmem added with a name longer than current SHMEM_INDEX_KEYSIZE.
And we don’t have such assertion now.
So, +1 for the patch.
Regards,
Zhang Mingli