From de239d01e7ccf7648e964d7a38c0f1c36bde8346 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 9 Sep 2024 14:34:10 -0400 Subject: [PATCH] Consistently use PageGetExactFreeSpace() in pgstattuple. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously this code used PageGetHeapFreeSpace on heap pages, and usually used PageGetFreeSpace on index pages (though for some reason GetHashPageStats used PageGetExactFreeSpace instead). The difference is that those functions subtract off the size of a line pointer, and PageGetHeapFreeSpace has some additional rules about returning zero if adding another line pointer would require exceeding MaxHeapTuplesPerPage. Those things make sense when testing to see if a new tuple can be put on the page, but they seem pretty strange for pure statistics collection. Additionally, statapprox_heap had a special rule about counting a "new" page as being fully available space. This also seems strange, because it's not actually usable until VACUUM or some such process initializes the page. Moreover, it's inconsistent with what pgstat_heap does, which is to count such a page as having zero free space. So make it work like pgstat_heap, which as of this patch unconditionally calls PageGetExactFreeSpace. This is more of a definitional change than a bug fix, so no back-patch. The module's documentation doesn't define exactly what "free space" means either, so we left that as-is. Frédéric Yhuel, reviewed by Rafia Sabih and Andreas Karlsson. Discussion: https://postgr.es/m/3a18f843-76f6-4a84-8cca-49537fefa15d@dalibo.com --- contrib/pgstattuple/pgstatapprox.c | 9 +-------- contrib/pgstattuple/pgstatindex.c | 2 +- contrib/pgstattuple/pgstattuple.c | 6 +++--- 3 files changed, 5 insertions(+), 12 deletions(-) diff --git a/contrib/pgstattuple/pgstatapprox.c b/contrib/pgstattuple/pgstatapprox.c index c84c642355..04457f4b79 100644 --- a/contrib/pgstattuple/pgstatapprox.c +++ b/contrib/pgstattuple/pgstatapprox.c @@ -106,14 +106,7 @@ statapprox_heap(Relation rel, output_type *stat) page = BufferGetPage(buf); - /* - * It's not safe to call PageGetHeapFreeSpace() on new pages, so we - * treat them as being free space for our purposes. - */ - if (!PageIsNew(page)) - stat->free_space += PageGetHeapFreeSpace(page); - else - stat->free_space += BLCKSZ - SizeOfPageHeaderData; + stat->free_space += PageGetExactFreeSpace(page); /* We may count the page as scanned even if it's new/empty */ scanned++; 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)) {