postgres.git / summary / log / commit / refs

commit    c0f9fe877e23ed5598efa9816b1eafeac3391756
Author:   Michael Paquier <michael@paquier.xyz>
Date:     Thu Oct 02 02:09:13 2025 +0000
First released in: 13.23, 14.20, 15.15, 16.11, 17.7, 18.1

    pgstattuple: Improve reports generated for indexes (hash, gist, btree)
    
    pgstattuple checks the state of the pages retrieved for gist and hash
    using some check functions from each index AM, respectively
    gistcheckpage() and _hash_checkpage().  When these are called, they
    would fail when bumping on data that is found as incorrect (like opaque
    area size not matching, or empty pages), contrary to btree that simply
    discards these cases and continues to aggregate data.
    
    Zero pages can happen after a crash, with these AMs being able to do an
    internal cleanup when these are seen.  Also, sporadic failures are
    annoying when doing for example a large-scale diagnostic query based on
    pgstattuple with a join of pg_class, as it forces one to use tricks like
    quals to discard hash or gist indexes, or use a PL wrapper able to catch
    errors.
    
    This commit changes the reports generated for btree, gist and hash to
    be more user-friendly;
    - When seeing an empty page, report it as free space.  This new rule
    applies to gist and hash, and already applied to btree.
    - For btree, a check based on the size of BTPageOpaqueData is added.
    - For gist indexes, gistcheckpage() is not called anymore, replaced by a
    check based on the size of GISTPageOpaqueData.
    - For hash indexes, instead of _hash_getbuf_with_strategy(), use a
    direct call to ReadBufferExtended(), coupled with a check based on
    HashPageOpaqueData.  The opaque area size check was already used.
    - Pages that do not match these criterias are discarded from the stats
    reports generated.
    
    There have been a couple of bug reports over the years that complained
    about the current behavior for hash and gist, as being not that useful,
    with nothing being done about it.  Hence this change is backpatched down
    to v13.
    
    Reported-by: Noah Misch <noah@leadboat.com>
    Author: Nitin Motiani <nitinmotiani@google.com>
    Reviewed-by: Dilip Kumar <dilipbalaut@gmail.com>
    Discussion: https://postgr.es/m/CAH5HC95gT1J3dRYK4qEnaywG8RqjbwDdt04wuj8p39R=HukayA@mail.gmail.com
    Backpatch-through: 13


 contrib/pgstattuple/pgstattuple.c | 32 ++++++++++++++++++++++----------
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/contrib/pgstattuple/pgstattuple.c b/contrib/pgstattuple/pgstattuple.c
index 1d012233cbf..a59fb42171e 100644
--- a/contrib/pgstattuple/pgstattuple.c
+++ b/contrib/pgstattuple/pgstattuple.c
@@ -422,7 +422,7 @@ pgstat_btree_page(pgstattuple_type *stat, Relation rel, BlockNumber blkno,
 		/* fully empty page */
 		stat->free_space += BLCKSZ;
 	}
-	else
+	else if (PageGetSpecialSize(page) == MAXALIGN(sizeof(BTPageOpaqueData)))
 	{
 		BTPageOpaque opaque;
 
@@ -456,10 +456,16 @@ pgstat_hash_page(pgstattuple_type *stat, Relation rel, BlockNumber blkno,
 	Buffer		buf;
 	Page		page;
 
-	buf = _hash_getbuf_with_strategy(rel, blkno, HASH_READ, 0, bstrategy);
+	buf = ReadBufferExtended(rel, MAIN_FORKNUM, blkno, RBM_NORMAL, bstrategy);
+	LockBuffer(buf, HASH_READ);
 	page = BufferGetPage(buf);
 
-	if (PageGetSpecialSize(page) == MAXALIGN(sizeof(HashPageOpaqueData)))
+	if (PageIsNew(page))
+	{
+		/* fully empty page */
+		stat->free_space += BLCKSZ;
+	}
+	else if (PageGetSpecialSize(page) == MAXALIGN(sizeof(HashPageOpaqueData)))
 	{
 		HashPageOpaque opaque;
 
@@ -500,17 +506,23 @@ pgstat_gist_page(pgstattuple_type *stat, Relation rel, BlockNumber blkno,
 
 	buf = ReadBufferExtended(rel, MAIN_FORKNUM, blkno, RBM_NORMAL, bstrategy);
 	LockBuffer(buf, GIST_SHARE);
-	gistcheckpage(rel, buf);
 	page = BufferGetPage(buf);
-
-	if (GistPageIsLeaf(page))
+	if (PageIsNew(page))
 	{
-		pgstat_index_page(stat, page, FirstOffsetNumber,
-						  PageGetMaxOffsetNumber(page));
+		/* fully empty page */
+		stat->free_space += BLCKSZ;
 	}
-	else
+	else if (PageGetSpecialSize(page) == MAXALIGN(sizeof(GISTPageOpaqueData)))
 	{
-		/* root or node */
+		if (GistPageIsLeaf(page))
+		{
+			pgstat_index_page(stat, page, FirstOffsetNumber,
+							  PageGetMaxOffsetNumber(page));
+		}
+		else
+		{
+			/* root or node */
+		}
 	}
 
 	UnlockReleaseBuffer(buf);


[parent: 36c4d30c8f24]