From 210916bcc236856e63b258a03573da5319b3e244 Mon Sep 17 00:00:00 2001 From: Bob Date: Sun, 15 Mar 2026 01:48:09 +0000 Subject: [PATCH 5/5] Use InvalidBlockNumber as empty marker This one required an update on simplehash implementation InvalidBlockNumber defined as ((BlockNumber) 0xFFFFFFFF) in ./src/include/storage/block.h by default SimpleHash simply zeroes the memory and that makes everything empty. For this case we have to call SH_MAKE_EMPTY on each entry after allocating. Removing the status field also a the hacks old_status = entry->status modify entry, corrupts status entry->status = old_status where the status initialized by simplehash had to be saved and restored when updating the entry. --- src/backend/nodes/tidbitmap.c | 19 ++++++------------- src/include/lib/simplehash.h | 22 ++++++++++++++++++++++ 2 files changed, 28 insertions(+), 13 deletions(-) diff --git a/src/backend/nodes/tidbitmap.c b/src/backend/nodes/tidbitmap.c index f1f925cb13..207f27f0ca 100644 --- a/src/backend/nodes/tidbitmap.c +++ b/src/backend/nodes/tidbitmap.c @@ -92,7 +92,6 @@ typedef struct PagetableEntry { BlockNumber blockno; /* page number (hashtable key) */ - char status; /* hash entry status */ bool ischunk; /* T = lossy storage, F = exact */ bool recheck; /* should the tuples be rechecked? */ bitmapword words[Max(WORDS_PER_PAGE, WORDS_PER_CHUNK)]; @@ -237,6 +236,12 @@ static int tbm_shared_comparator(const void *left, const void *right, #define SH_KEY blockno #define SH_HASH_KEY(tb, key) murmurhash32(key) #define SH_EQUAL(tb, a, b) a == b +#define SH_ENTRY_EMPTY(entry) ((entry)->blockno == InvalidBlockNumber) +#define SH_MAKE_EMPTY(entry) ((entry)->blockno = InvalidBlockNumber) +#define SH_MAKE_IN_USE(entry) ((void)0) +// Since the empty marker is non-zero, we need to reset the entries +// after allocation using the custom SH_MAKE_EMPTY macro. +#define SH_NONZERO_EMPTY #define SH_SCOPE static inline #define SH_DEFINE #define SH_DECLARE @@ -291,15 +296,12 @@ tbm_create_pagetable(TIDBitmap *tbm) { PagetableEntry *page; bool found; - char oldstatus; page = pagetable_insert(tbm->pagetable, tbm->entry1.blockno, &found); Assert(!found); - oldstatus = page->status; memcpy(page, &tbm->entry1, sizeof(PagetableEntry)); - page->status = oldstatus; } tbm->status = TBM_HASH; @@ -1230,10 +1232,7 @@ tbm_get_pageentry(TIDBitmap *tbm, BlockNumber pageno) /* Initialize it if not present before */ if (!found) { - char oldstatus = page->status; - MemSet(page, 0, sizeof(PagetableEntry)); - page->status = oldstatus; page->blockno = pageno; /* must count it too */ tbm->nentries++; @@ -1317,10 +1316,7 @@ tbm_mark_page_lossy(TIDBitmap *tbm, BlockNumber pageno) /* Initialize it if not present before */ if (!found) { - char oldstatus = page->status; - MemSet(page, 0, sizeof(PagetableEntry)); - page->status = oldstatus; page->blockno = chunk_pageno; page->ischunk = true; /* must count it too */ @@ -1329,11 +1325,8 @@ tbm_mark_page_lossy(TIDBitmap *tbm, BlockNumber pageno) } else if (!page->ischunk) { - char oldstatus = page->status; - /* chunk header page was formerly non-lossy, make it lossy */ MemSet(page, 0, sizeof(PagetableEntry)); - page->status = oldstatus; page->blockno = chunk_pageno; page->ischunk = true; /* we assume it had some tuple bit(s) set, so mark it lossy */ diff --git a/src/include/lib/simplehash.h b/src/include/lib/simplehash.h index 3c03a7e9c9..c890a09b76 100644 --- a/src/include/lib/simplehash.h +++ b/src/include/lib/simplehash.h @@ -301,6 +301,12 @@ SH_SCOPE void SH_STAT(SH_TYPE * tb); #define SH_MAKE_IN_USE(entry) ((entry)->status = SH_STATUS_IN_USE) #endif +/* + * If the empty marker is non-zero (e.g., InvalidBlockNumber = 0xFFFFFFFF), + * define SH_NONZERO_EMPTY to explicitly initialize entries. When unset, + * zero-initialization via memset is sufficient (the default). + */ + /* * Wrap the following definitions in include guards, to avoid multiple * definition errors if this header is included more than once. The rest of @@ -481,6 +487,11 @@ SH_CREATE(MemoryContext ctx, uint32 nelements, void *private_data) tb->data = (SH_ELEMENT_TYPE *) SH_ALLOCATE(tb, sizeof(SH_ELEMENT_TYPE) * size); +#ifdef SH_NONZERO_EMPTY + for (uint64 i = 0; i < size; i++) + SH_MAKE_EMPTY(&tb->data[i]); +#endif + SH_UPDATE_PARAMETERS(tb, size); return tb; } @@ -497,7 +508,12 @@ SH_DESTROY(SH_TYPE * tb) SH_SCOPE void SH_RESET(SH_TYPE * tb) { +#ifdef SH_NONZERO_EMPTY + for (uint32 i = 0; i < tb->size; i++) + SH_MAKE_EMPTY(&tb->data[i]); +#else memset(tb->data, 0, sizeof(SH_ELEMENT_TYPE) * tb->size); +#endif tb->members = 0; } @@ -526,6 +542,11 @@ SH_GROW(SH_TYPE * tb, uint64 newsize) tb->data = (SH_ELEMENT_TYPE *) SH_ALLOCATE(tb, sizeof(SH_ELEMENT_TYPE) * newsize); +#ifdef SH_NONZERO_EMPTY + for (uint64 j = 0; j < newsize; j++) + SH_MAKE_EMPTY(&tb->data[j]); +#endif + /* * Update parameters for new table after allocation succeeds to avoid * inconsistent state on OOM. @@ -1222,6 +1243,7 @@ SH_STAT(SH_TYPE * tb) #undef SH_ENTRY_EMPTY #undef SH_MAKE_EMPTY #undef SH_MAKE_IN_USE +#undef SH_NONZERO_EMPTY /* undefine locally declared macros */ #undef SH_MAKE_PREFIX -- 2.34.1