Bogus bitmasking in heap2_desc

Started by Julien Rouhaudabout 1 year ago4 messageshackers
Beta feature

Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.

appliessuccessCI history

You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:

docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t51994
psql -h localhost -U postgres

Built from patchset v1 (message #1), September 20, 2026 at 03:12 AM.

Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:

git clone --branch t51994_1 https://github.com/hackorum-dev/postgres.git

In a checkout you already have, add the fork once:

git remote add hackorum https://github.com/hackorum-dev/postgres.git

then, for this patchset and every later one:

git fetch hackorum t51994_1 && git checkout t51994_1

Patchset v1 (message #1) is on t51994_1

Jump to latest
#1Julien Rouhaud
rjuju123@gmail.com

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:

t51994_1
v1-0001-Remove-bogus-bitmask-in-heap2_desc.patchtext/plain; charset=us-asciiDownload+0-2
#2Michael Paquier
michael@paquier.xyz
In reply to: Julien Rouhaud (#1)
Re: 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

#3Julien Rouhaud
rjuju123@gmail.com
In reply to: Michael Paquier (#2)
Re: Bogus bitmasking in heap2_desc

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.

#4Michael Paquier
michael@paquier.xyz
In reply to: Julien Rouhaud (#3)
Re: Bogus bitmasking in heap2_desc

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