From 48e6c943e5f11f5d80cabdbcd98f79e3dbad1988 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 18 Feb 2016 15:40:35 -0500 Subject: [PATCH] Fix multiple bugs in contrib/pgstattuple's pgstatindex() function. Dead or half-dead index leaf pages were incorrectly reported as live, as a consequence of a code rearrangement I made (during a moment of severe brain fade, evidently) in commit d287818eb514d431. The index metapage was not counted in index_size, causing that result to not agree with the actual index size on-disk. Index root pages were not counted in internal_pages, which is inconsistent compared to the case of a root that's also a leaf (one-page index), where the root would be counted in leaf_pages. Aside from that inconsistency, this could lead to additional transient discrepancies between the reported page counts and index_size, since it's possible for pgstatindex's scan to see zero or multiple pages marked as BTP_ROOT, if the root moves due to a split during the scan. With these fixes, index_size will always be exactly one page more than the sum of the displayed page counts. Also, the index_size result was incorrectly documented as being measured in pages; it's always been measured in bytes. (While fixing that, I couldn't resist doing some small additional wordsmithing on the pgstattuple docs.) Including the metapage causes the reported index_size to not be zero for an empty index. To preserve the desired property that the pgstattuple regression test results are platform-independent (ie, BLCKSZ configuration independent), scale the index_size result in the regression tests. The documentation issue was reported by Otsuka Kenji, and the inconsistent root page counting by Peter Geoghegan; the other problems noted by me. Back-patch to all supported branches, because this has been broken for a long time. --- contrib/pgstattuple/expected/pgstattuple.out | 44 +++++++++++--------- contrib/pgstattuple/pgstatindex.c | 16 +++---- contrib/pgstattuple/sql/pgstattuple.sql | 26 +++++++++--- doc/src/sgml/pgstattuple.sgml | 29 ++++++++----- 4 files changed, 68 insertions(+), 47 deletions(-) diff --git a/contrib/pgstattuple/expected/pgstattuple.out b/contrib/pgstattuple/expected/pgstattuple.out index d769f6d494..e920234488 100644 --- a/contrib/pgstattuple/expected/pgstattuple.out +++ b/contrib/pgstattuple/expected/pgstattuple.out @@ -41,40 +41,44 @@ select pgstattuple(relname) from pg_class where relname = 'test'; (0,0,0,0,0,0,0,0,0) (1 row) -select * from pgstatindex('test_pkey'); +select version, tree_level, + index_size / current_setting('block_size')::int as index_size, + root_block_no, internal_pages, leaf_pages, empty_pages, deleted_pages, + avg_leaf_density, leaf_fragmentation + from pgstatindex('test_pkey'); version | tree_level | index_size | root_block_no | internal_pages | leaf_pages | empty_pages | deleted_pages | avg_leaf_density | leaf_fragmentation ---------+------------+------------+---------------+----------------+------------+-------------+---------------+------------------+-------------------- - 2 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | NaN | NaN + 2 | 0 | 1 | 0 | 0 | 0 | 0 | 0 | NaN | NaN (1 row) -select * from pgstatindex('test_pkey'::text); +select version, tree_level, + index_size / current_setting('block_size')::int as index_size, + root_block_no, internal_pages, leaf_pages, empty_pages, deleted_pages, + avg_leaf_density, leaf_fragmentation + from pgstatindex('test_pkey'::text); version | tree_level | index_size | root_block_no | internal_pages | leaf_pages | empty_pages | deleted_pages | avg_leaf_density | leaf_fragmentation ---------+------------+------------+---------------+----------------+------------+-------------+---------------+------------------+-------------------- - 2 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | NaN | NaN + 2 | 0 | 1 | 0 | 0 | 0 | 0 | 0 | NaN | NaN (1 row) -select * from pgstatindex('test_pkey'::name); +select version, tree_level, + index_size / current_setting('block_size')::int as index_size, + root_block_no, internal_pages, leaf_pages, empty_pages, deleted_pages, + avg_leaf_density, leaf_fragmentation + from pgstatindex('test_pkey'::name); version | tree_level | index_size | root_block_no | internal_pages | leaf_pages | empty_pages | deleted_pages | avg_leaf_density | leaf_fragmentation ---------+------------+------------+---------------+----------------+------------+-------------+---------------+------------------+-------------------- - 2 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | NaN | NaN + 2 | 0 | 1 | 0 | 0 | 0 | 0 | 0 | NaN | NaN (1 row) -select * from pgstatindex('test_pkey'::regclass); +select version, tree_level, + index_size / current_setting('block_size')::int as index_size, + root_block_no, internal_pages, leaf_pages, empty_pages, deleted_pages, + avg_leaf_density, leaf_fragmentation + from pgstatindex('test_pkey'::regclass); version | tree_level | index_size | root_block_no | internal_pages | leaf_pages | empty_pages | deleted_pages | avg_leaf_density | leaf_fragmentation ---------+------------+------------+---------------+----------------+------------+-------------+---------------+------------------+-------------------- - 2 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | NaN | NaN -(1 row) - -select pgstatindex(oid) from pg_class where relname = 'test_pkey'; - pgstatindex ---------------------------- - (2,0,0,0,0,0,0,0,NaN,NaN) -(1 row) - -select pgstatindex(relname) from pg_class where relname = 'test_pkey'; - pgstatindex ---------------------------- - (2,0,0,0,0,0,0,0,NaN,NaN) + 2 | 0 | 1 | 0 | 0 | 0 | 0 | 0 | NaN | NaN (1 row) select pg_relpages('test'); diff --git a/contrib/pgstattuple/pgstatindex.c b/contrib/pgstattuple/pgstatindex.c index 8048a2cbe7..9f1377c4cd 100644 --- a/contrib/pgstattuple/pgstatindex.c +++ b/contrib/pgstattuple/pgstatindex.c @@ -79,7 +79,6 @@ typedef struct BTIndexStat uint32 level; BlockNumber root_blkno; - uint64 root_pages; uint64 internal_pages; uint64 leaf_pages; uint64 empty_pages; @@ -185,7 +184,6 @@ pgstatindex_impl(Relation rel, FunctionCallInfo fcinfo) } /* -- init counters -- */ - indexStat.root_pages = 0; indexStat.internal_pages = 0; indexStat.leaf_pages = 0; indexStat.empty_pages = 0; @@ -218,7 +216,11 @@ pgstatindex_impl(Relation rel, FunctionCallInfo fcinfo) /* Determine page type, and update totals */ - if (P_ISLEAF(opaque)) + if (P_ISDELETED(opaque)) + indexStat.deleted_pages++; + else if (P_IGNORE(opaque)) + indexStat.empty_pages++; /* this is the "half dead" state */ + else if (P_ISLEAF(opaque)) { int max_avail; @@ -235,12 +237,6 @@ pgstatindex_impl(Relation rel, FunctionCallInfo fcinfo) if (opaque->btpo_next != P_NONE && opaque->btpo_next < blkno) indexStat.fragments++; } - else if (P_ISDELETED(opaque)) - indexStat.deleted_pages++; - else if (P_IGNORE(opaque)) - indexStat.empty_pages++; - else if (P_ISROOT(opaque)) - indexStat.root_pages++; else indexStat.internal_pages++; @@ -269,7 +265,7 @@ pgstatindex_impl(Relation rel, FunctionCallInfo fcinfo) values[j++] = psprintf("%d", indexStat.version); values[j++] = psprintf("%d", indexStat.level); values[j++] = psprintf(INT64_FORMAT, - (indexStat.root_pages + + (1 + /* include the metapage in index_size */ indexStat.leaf_pages + indexStat.internal_pages + indexStat.deleted_pages + diff --git a/contrib/pgstattuple/sql/pgstattuple.sql b/contrib/pgstattuple/sql/pgstattuple.sql index 0e0ad0e15d..d22c9f1c46 100644 --- a/contrib/pgstattuple/sql/pgstattuple.sql +++ b/contrib/pgstattuple/sql/pgstattuple.sql @@ -15,12 +15,26 @@ select * from pgstattuple('test'::regclass); select pgstattuple(oid) from pg_class where relname = 'test'; select pgstattuple(relname) from pg_class where relname = 'test'; -select * from pgstatindex('test_pkey'); -select * from pgstatindex('test_pkey'::text); -select * from pgstatindex('test_pkey'::name); -select * from pgstatindex('test_pkey'::regclass); -select pgstatindex(oid) from pg_class where relname = 'test_pkey'; -select pgstatindex(relname) from pg_class where relname = 'test_pkey'; +select version, tree_level, + index_size / current_setting('block_size')::int as index_size, + root_block_no, internal_pages, leaf_pages, empty_pages, deleted_pages, + avg_leaf_density, leaf_fragmentation + from pgstatindex('test_pkey'); +select version, tree_level, + index_size / current_setting('block_size')::int as index_size, + root_block_no, internal_pages, leaf_pages, empty_pages, deleted_pages, + avg_leaf_density, leaf_fragmentation + from pgstatindex('test_pkey'::text); +select version, tree_level, + index_size / current_setting('block_size')::int as index_size, + root_block_no, internal_pages, leaf_pages, empty_pages, deleted_pages, + avg_leaf_density, leaf_fragmentation + from pgstatindex('test_pkey'::name); +select version, tree_level, + index_size / current_setting('block_size')::int as index_size, + root_block_no, internal_pages, leaf_pages, empty_pages, deleted_pages, + avg_leaf_density, leaf_fragmentation + from pgstatindex('test_pkey'::regclass); select pg_relpages('test'); select pg_relpages('test_pkey'); diff --git a/doc/src/sgml/pgstattuple.sgml b/doc/src/sgml/pgstattuple.sgml index 18d244ba3a..61340bedbc 100644 --- a/doc/src/sgml/pgstattuple.sgml +++ b/doc/src/sgml/pgstattuple.sgml @@ -130,9 +130,9 @@ free_percent | 1.95 This is the same as pgstattuple(regclass), except - that the target relation is specified by TEXT. This function is kept + that the target relation is specified as TEXT. This function is kept because of backward-compatibility so far, and will be deprecated in - the future release. + some future release. @@ -154,13 +154,13 @@ test=> SELECT * FROM pgstatindex('pg_cast_oid_index'); -[ RECORD 1 ]------+------ version | 2 tree_level | 0 -index_size | 8192 +index_size | 16384 root_block_no | 1 internal_pages | 0 leaf_pages | 1 empty_pages | 0 deleted_pages | 0 -avg_leaf_density | 50.27 +avg_leaf_density | 54.27 leaf_fragmentation | 0 @@ -194,13 +194,13 @@ leaf_fragmentation | 0 index_size bigint - Total number of pages in index + Total index size in bytes root_block_no bigint - Location of root block + Location of root page (zero if none) @@ -244,6 +244,13 @@ leaf_fragmentation | 0 + + The reported index_size will normally correspond to one more + page than is accounted for by internal_pages + leaf_pages + + empty_pages + deleted_pages, because it also includes the + index's metapage. + + As with pgstattuple, the results are accumulated page-by-page, and should not be expected to represent an @@ -260,9 +267,9 @@ leaf_fragmentation | 0 This is the same as pgstatindex(regclass), except - that the target index is specified by TEXT. This function is kept + that the target index is specified as TEXT. This function is kept because of backward-compatibility so far, and will be deprecated in - the future release. + some future release. @@ -351,9 +358,9 @@ pending_tuples | 0 This is the same as pg_relpages(regclass), except - that the target relation is specified by TEXT. This function is kept + that the target relation is specified as TEXT. This function is kept because of backward-compatibility so far, and will be deprecated in - the future release. + some future release. @@ -370,7 +377,7 @@ pending_tuples | 0 pgstattuple_approx is a faster alternative to pgstattuple that returns approximate results. - The argument is the target relation's OID. + The argument is the target relation's name or OID. For example: test=> SELECT * FROM pgstattuple_approx('pg_catalog.pg_proc'::regclass);