Consistently use PageGetExactFreeSpace() in pgstattuple.

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

diff --git a/contrib/pgstattuple/pgstatapprox.c b/contrib/pgstattuple/pgstatapprox.c
index c84c6423555e2d24a275b4b7c7cd336e19c614b1..04457f4b790d5456fa82cc7d1dff39e81e40fe02 100644 (file)
--- 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 5c06ba6db438e9205ba96a59ca9fa09bef0b1ecc..1b6b768cf80e75b5ba1fdc3b7c6d0de6e68a6507 100644 (file)
--- 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 3bd8b96197f86d1b3df9c26c8671825177e9aaf2..7e2a7262a35c75a50f8399ef2279cf10a9b0b37a 100644 (file)
--- 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))
    {