Make gistvacuumcleanup() count the actual number of index tuples.
authorTom Lane
Fri, 2 Mar 2018 16:22:42 +0000 (11:22 -0500)
committerTom Lane
Fri, 2 Mar 2018 16:22:42 +0000 (11:22 -0500)
Previously, it just returned the heap tuple count, which might be only an
estimate, and would be completely the wrong thing if the index is partial.
Since this function scans every index page anyway to find free pages,
it's practically free to count the surviving index tuples.  Let's do that
and return an accurate count.

This is easily visible as a wrong reltuples value for a partial GiST
index following VACUUM, so back-patch to all supported branches.

Andrey Borodin, reviewed by Michail Nikolaev

Discussion: https://postgr.es/m/151956654251.6915.675951950408204404[email protected]

src/backend/access/gist/gistvacuum.c

index 2667fc3ea25a159bfa461a26fd425e8c5debd824..4335419096c2975e3e4b32fd67519a5fd9793e31 100644 (file)
@@ -34,6 +34,7 @@ gistvacuumcleanup(PG_FUNCTION_ARGS)
    BlockNumber npages,
                blkno;
    BlockNumber totFreePages;
+   double      tuplesCount;
    bool        needLock;
 
    /* No-op in ANALYZE ONLY mode */
@@ -42,17 +43,7 @@ gistvacuumcleanup(PG_FUNCTION_ARGS)
 
    /* Set up all-zero stats if gistbulkdelete wasn't called */
    if (stats == NULL)
-   {
        stats = (IndexBulkDeleteResult *) palloc0(sizeof(IndexBulkDeleteResult));
-       /* use heap's tuple count */
-       stats->num_index_tuples = info->num_heap_tuples;
-       stats->estimated_count = info->estimated_count;
-
-       /*
-        * XXX the above is wrong if index is partial.  Would it be OK to just
-        * return NULL, or is there work we must do below?
-        */
-   }
 
    /*
     * Need lock unless it's local to this backend.
@@ -67,6 +58,7 @@ gistvacuumcleanup(PG_FUNCTION_ARGS)
        UnlockRelationForExtension(rel, ExclusiveLock);
 
    totFreePages = 0;
+   tuplesCount = 0;
    for (blkno = GIST_ROOT_BLKNO + 1; blkno < npages; blkno++)
    {
        Buffer      buffer;
@@ -84,6 +76,11 @@ gistvacuumcleanup(PG_FUNCTION_ARGS)
            totFreePages++;
            RecordFreeIndexPage(rel, blkno);
        }
+       else if (GistPageIsLeaf(page))
+       {
+           /* count tuples in index (considering only leaf tuples) */
+           tuplesCount += PageGetMaxOffsetNumber(page);
+       }
        UnlockReleaseBuffer(buffer);
    }
 
@@ -97,6 +94,8 @@ gistvacuumcleanup(PG_FUNCTION_ARGS)
    stats->num_pages = RelationGetNumberOfBlocks(rel);
    if (needLock)
        UnlockRelationForExtension(rel, ExclusiveLock);
+   stats->num_index_tuples = tuplesCount;
+   stats->estimated_count = false;
 
    PG_RETURN_POINTER(stats);
 }