Remove unneeded check for XLH_INSERT_ALL_FROZEN in heap_xlog_insert
Hi,
I noticed that 8e03eb92e9a forgot one line when reverting 39b66a91bd.
There is an extraneous check for XLH_INSERT_ALL_FROZEN_SET in
heap_xlog_insert() -- even though heap_insert() never freezes tuples.
It doesn't hurt anything, but I found it confusing, so I think it is
worth removing. I don't think it's worth backpatching, so I don't know
if that means that this commit shouldn't go in master until after we
branch.
- Melanie
Attachments:
v1-0001-Remove-unused-check-in-heap_xlog_insert.patchtext/x-patch; charset=US-ASCII; name=v1-0001-Remove-unused-check-in-heap_xlog_insert.patchDownload
From 96e9896bd737c9e2a647f5cb04465bfeb6d0c041 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Tue, 24 Jun 2025 13:32:15 -0400
Subject: [PATCH v1] Remove unused check in heap_xlog_insert()
8e03eb92e9ad54e2 reverted the commit 39b66a91bd which allowed freezing
in the heap_insert() code path but forgot to remove the corresponding
check in heap_xlog_insert(). This code is extraneous but not harmful.
However, cleaning it up makes it very clear that, as of now, we do not
support any freezing of pages in the heap_insert() path.
---
src/backend/access/heap/heapam_xlog.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/backend/access/heap/heapam_xlog.c b/src/backend/access/heap/heapam_xlog.c
index 30f4c2d3c67..eb4bd3d6ae3 100644
--- a/src/backend/access/heap/heapam_xlog.c
+++ b/src/backend/access/heap/heapam_xlog.c
@@ -438,6 +438,9 @@ heap_xlog_insert(XLogReaderState *record)
ItemPointerSetBlockNumber(&target_tid, blkno);
ItemPointerSetOffsetNumber(&target_tid, xlrec->offnum);
+ /* No freezing in the heap_insert() code path */
+ Assert(!(xlrec->flags & XLH_INSERT_ALL_FROZEN_SET));
+
/*
* The visibility map may need to be fixed even if the heap page is
* already up-to-date.
@@ -508,10 +511,6 @@ heap_xlog_insert(XLogReaderState *record)
if (xlrec->flags & XLH_INSERT_ALL_VISIBLE_CLEARED)
PageClearAllVisible(page);
- /* XLH_INSERT_ALL_FROZEN_SET implies that all tuples are visible */
- if (xlrec->flags & XLH_INSERT_ALL_FROZEN_SET)
- PageSetAllVisible(page);
-
MarkBufferDirty(buffer);
}
if (BufferIsValid(buffer))
--
2.34.1
On 6/24/25 19:41, Melanie Plageman wrote:
Hi,
I noticed that 8e03eb92e9a forgot one line when reverting 39b66a91bd.
There is an extraneous check for XLH_INSERT_ALL_FROZEN_SET in
heap_xlog_insert() -- even though heap_insert() never freezes tuples.
It doesn't hurt anything, but I found it confusing, so I think it is
worth removing. I don't think it's worth backpatching, so I don't know
if that means that this commit shouldn't go in master until after we
branch.
Thanks for noticing this. I'd probably be +0.5 to backpatch this, even
if it's ultimately harmless, just to keep the code not confusing.
regards
--
Tomas Vondra
On Tue, Jun 24, 2025 at 1:53 PM Tomas Vondra <tomas@vondra.me> wrote:
Thanks for noticing this. I'd probably be +0.5 to backpatch this, even
if it's ultimately harmless, just to keep the code not confusing.
Okay, I can backpatch this (to 14, I believe). I'll wait until the
thread has been around for closer to 24 hours before doing anything,
though.
- Melanie
On Tue, Jun 24, 2025 at 4:48 PM Melanie Plageman
<melanieplageman@gmail.com> wrote:
Okay, I can backpatch this (to 14, I believe). I'll wait until the
thread has been around for closer to 24 hours before doing anything,
though.
Pushed and back-patched through 14.
- Melanie