On 8/22/24 21:56, Rafia Sabih wrote:
I agree with the approach here.
A minor comment here is to change the comments in code referring to the
PageGetHeapFreeSpace.
Thank you Rafia. Here is a v2 patch.
I've also added this to the commit message:
Also, PageGetHeapFreeSpace() will return zero if there are already
MaxHeapTuplesPerPage line pointers in the page and none are free. We
don't want that either, because here we want to keep track of the free
space after a page pruning operation even in the (very unlikely) case
that there are MaxHeapTuplesPerPage line pointers in the page.From c4d0569cd3dbcec7f84a794e927cfc81e137ff6a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Yhuel?= <frederic.yh...@dalibo.com>
Date: Wed, 21 Aug 2024 15:05:11 +0200
Subject: [PATCH v2] pgstattuple: use PageGetExactFreeSpace()
instead of PageGetHeapFreeSpace() or PageGetFreeSpace()
We want the exact free space, we don't care if there's enough room for
another line pointer.
Also, PageGetHeapFreeSpace() will return zero if there are already
MaxHeapTuplesPerPage line pointers in the page and none are free. We
don't want that either, because here we want to keep track of the free
space after a page pruning operation even in the (very unlikely) case
that there are MaxHeapTuplesPerPage line pointers in the page.
---
contrib/pgstattuple/pgstatapprox.c | 4 ++--
contrib/pgstattuple/pgstatindex.c | 2 +-
contrib/pgstattuple/pgstattuple.c | 6 +++---
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/contrib/pgstattuple/pgstatapprox.c b/contrib/pgstattuple/pgstatapprox.c
index c84c642355..b63a9932d7 100644
--- a/contrib/pgstattuple/pgstatapprox.c
+++ b/contrib/pgstattuple/pgstatapprox.c
@@ -107,11 +107,11 @@ statapprox_heap(Relation rel, output_type *stat)
page = BufferGetPage(buf);
/*
- * It's not safe to call PageGetHeapFreeSpace() on new pages, so we
+ * It's not safe to call PageGetExactFreeSpace() on new pages, so we
* treat them as being free space for our purposes.
*/
if (!PageIsNew(page))
- stat->free_space += PageGetHeapFreeSpace(page);
+ stat->free_space += PageGetExactFreeSpace(page);
else
stat->free_space += BLCKSZ - SizeOfPageHeaderData;
diff --git a/contrib/pgstattuple/pgstatindex.c b/contrib/pgstattuple/pgstatindex.c
index 5c06ba6db4..1b6b768cf8 100644
--- a/contrib/pgstattuple/pgstatindex.c
+++ b/contrib/pgstattuple/pgstatindex.c
@@ -311,7 +311,7 @@ pgstatindex_impl(Relation rel, FunctionCallInfo fcinfo)
max_avail = BLCKSZ - (BLCKSZ - ((PageHeader) page)->pd_special + SizeOfPageHeaderData);
indexStat.max_avail += max_avail;
- indexStat.free_space += PageGetFreeSpace(page);
+ indexStat.free_space += PageGetExactFreeSpace(page);
indexStat.leaf_pages++;
diff --git a/contrib/pgstattuple/pgstattuple.c b/contrib/pgstattuple/pgstattuple.c
index 3bd8b96197..7e2a7262a3 100644
--- a/contrib/pgstattuple/pgstattuple.c
+++ b/contrib/pgstattuple/pgstattuple.c
@@ -372,7 +372,7 @@ pgstat_heap(Relation rel, FunctionCallInfo fcinfo)
buffer = ReadBufferExtended(rel, MAIN_FORKNUM, block,
RBM_NORMAL, hscan->rs_strategy);
LockBuffer(buffer, BUFFER_LOCK_SHARE);
- stat.free_space += PageGetHeapFreeSpace((Page) BufferGetPage(buffer));
+ stat.free_space += PageGetExactFreeSpace((Page) BufferGetPage(buffer));
UnlockReleaseBuffer(buffer);
block++;
}
@@ -385,7 +385,7 @@ pgstat_heap(Relation rel, FunctionCallInfo fcinfo)
buffer = ReadBufferExtended(rel, MAIN_FORKNUM, block,
RBM_NORMAL, hscan->rs_strategy);
LockBuffer(buffer, BUFFER_LOCK_SHARE);
- stat.free_space += PageGetHeapFreeSpace((Page) BufferGetPage(buffer));
+ stat.free_space += PageGetExactFreeSpace((Page) BufferGetPage(buffer));
UnlockReleaseBuffer(buffer);
block++;
}
@@ -565,7 +565,7 @@ pgstat_index_page(pgstattuple_type *stat, Page page,
{
OffsetNumber i;
- stat->free_space += PageGetFreeSpace(page);
+ stat->free_space += PageGetExactFreeSpace(page);
for (i = minoff; i <= maxoff; i = OffsetNumberNext(i))
{
--
2.39.2
