Bogus bitmasking in heap2_desc
Hi,
heap2_desc apparently inherited the extra bit filtering from heap_desc to
ignore XLOG_HEAP_INIT_PAGE. But XLOG_HEAP2 only has real opcodes in the
high bits there is no reason why there should be this filtering, and even if it
eventually needed additional filtering we would need something specific to this
resource manager anyway, so I think we can get rid of it as in the attached.
Attachments:
v1-0001-Remove-bogus-bitmask-in-heap2_desc.patchtext/plain; charset=us-asciiDownload
From bc497a2440bdec38d0abd45fa729506491fb8baf Mon Sep 17 00:00:00 2001
From: Julien Rouhaud <julien.rouhaud@free.fr>
Date: Sun, 27 Jul 2025 17:10:54 +0800
Subject: [PATCH v1] Remove bogus bitmask in heap2_desc
Unlike XLOG_HEAP, XLOG_HEAP2 resource manager doesn't have a bit used for
something other than an opcode, so there is no need to do additional masking on
top of the usual XLR_INFO_MASK, especially not one based on XLOG_HEAP list of
opcodes.
---
src/backend/access/rmgrdesc/heapdesc.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/backend/access/rmgrdesc/heapdesc.c b/src/backend/access/rmgrdesc/heapdesc.c
index 82b62c95de5..cc637636141 100644
--- a/src/backend/access/rmgrdesc/heapdesc.c
+++ b/src/backend/access/rmgrdesc/heapdesc.c
@@ -266,7 +266,6 @@ heap2_desc(StringInfo buf, XLogReaderState *record)
char *rec = XLogRecGetData(record);
uint8 info = XLogRecGetInfo(record) & ~XLR_INFO_MASK;
- info &= XLOG_HEAP_OPMASK;
if (info == XLOG_HEAP2_PRUNE_ON_ACCESS ||
info == XLOG_HEAP2_PRUNE_VACUUM_SCAN ||
info == XLOG_HEAP2_PRUNE_VACUUM_CLEANUP)
--
2.50.1
On Sun, Jul 27, 2025 at 05:19:56PM +0800, Julien Rouhaud wrote:
heap2_desc apparently inherited the extra bit filtering from heap_desc to
ignore XLOG_HEAP_INIT_PAGE. But XLOG_HEAP2 only has real opcodes in the
high bits there is no reason why there should be this filtering, and even if it
eventually needed additional filtering we would need something specific to this
resource manager anyway, so I think we can get rid of it as in the attached.
@@ -266,7 +266,6 @@ heap2_desc(StringInfo buf, XLogReaderState *record)
char *rec = XLogRecGetData(record);
uint8 info = XLogRecGetInfo(record) & ~XLR_INFO_MASK;
- info &= XLOG_HEAP_OPMASK;
if (info == XLOG_HEAP2_PRUNE_ON_ACCESS ||
info == XLOG_HEAP2_PRUNE_VACUUM_SCAN ||
info == XLOG_HEAP2_PRUNE_VACUUM_CLEANUP)
The relationship between XLOG_HEAP_OPMASK and heap2 is documented in
heapam_xlog.h. XLOG_HEAP2_MULTI_INSERT may have XLOG_HEAP_INIT_PAGE
set, so if we don't filter the contents from XLogRecGetInfo() then the
record description becomes incorrect for the XLOG_HEAP2_MULTI_INSERT
"MULTI_INSERT+INIT" case, no?
Apologies if I am missing your point.
--
Michael
Hi,
On Mon, Jul 28, 2025 at 02:27:43PM +0900, Michael Paquier wrote:
The relationship between XLOG_HEAP_OPMASK and heap2 is documented in
heapam_xlog.h. XLOG_HEAP2_MULTI_INSERT may have XLOG_HEAP_INIT_PAGE
set, so if we don't filter the contents from XLogRecGetInfo() then the
record description becomes incorrect for the XLOG_HEAP2_MULTI_INSERT
"MULTI_INSERT+INIT" case, no?Apologies if I am missing your point.
Ah I'm the one apologizing, I totally missed that comment.
Thanks for the pointing it out.