Bogus bitmasking in heap2_desc

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.

On Mon, Jul 28, 2025 at 01:31:15PM +0800, Julien Rouhaud wrote:

Ah I'm the one apologizing, I totally missed that comment.

Thanks for the pointing it out.

Thanks, no problem.
--
Michael