On 8/23/24 12:51, Frédéric Yhuel wrote:
On 8/23/24 12:02, Rafia Sabih wrote:
On the other hand, this got me thinking about the purpose of this
space information.
If we want to understand that there's still some space for the tuples
in a page, then using PageGetExactFreeSpace is not doing justice in
case of heap page, because we will not be able to add any more tuples
there if there are already MaxHeapTuplesPerPage tuples there.
We won't be able to add, but we will be able to update a tuple in this
page.
Sorry, that's not true.
So in this marginal case we have free space that's unusable in practice.
No INSERT or UPDATE (HOT or not) is possible inside the page.
I don't know what pgstattuple should do in this case.
However, we should never encounter this case in practice (maybe on some
exotic architectures with strange alignment behavior?). As I said, I
can't fit more than 226 tuples per page on my machine, while
MaxHeapTuplesPerPage is 291. Am I missing something?
Besides, pgstattuple isn't mission critical, is it?
So I think we should just use PageGetExactFreeSpace().
Here is a v3 patch. It's the same as v2, I only removed the last
paragraph in the commit message.
Thank you Rafia and Andreas for your review and test.
Best regards,
Frédéric
From f749d4ca2d6881a916c6ca2a3b882bb2740188d4 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 v3] 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.
---
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
