Fix a missed case in code for "moving average" estimate of reltuples.

It is possible for VACUUM to scan no pages at all, if the visibility map
shows that all pages are all-visible.  In this situation VACUUM has no new
information to report about the relation's tuple density, so it wasn't
changing pg_class.reltuples ... but it updated pg_class.relpages anyway.
That's wrong in general, since there is no evidence to justify changing the
density ratio reltuples/relpages, but it's particularly bad if the previous
state was relpages=reltuples=0, which means "unknown tuple density".
We just replaced "unknown" with "zero".  ANALYZE would eventually recover
from this, but it could take a lot of repetitions of ANALYZE to do so if
the relation size is much larger than the maximum number of pages ANALYZE
will scan, because of the moving-average behavior introduced by commit
b4b6923e03f4d29636a94f6f4cc2f5cf6298b8c8.

The only known situation where we could have relpages=reltuples=0 and yet
the visibility map asserts everything's visible is immediately following
a pg_upgrade.  It might be advisable for pg_upgrade to try to preserve the
relpages/reltuples statistics; but in any case this code is wrong on its
own terms, so fix it.  Per report from Sergey Koposov.

Back-patch to 8.4, where the visibility map was introduced, same as the
previous change.

diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index e6a7cdcf901071841098bdaedb33f9605c2fbacb..bdff22e451af85a5788e2f2eb6c9f1edc5ea46b7 100644 (file)
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -714,8 +714,10 @@ vac_estimate_reltuples(Relation relation, bool is_analyze,
                return scanned_tuples;
 
        /*
-        * If scanned_pages is zero but total_pages isn't, keep the existing
-        * value of reltuples.
+        * If scanned_pages is zero but total_pages isn't, keep the existing value
+        * of reltuples.  (Note: callers should avoid updating the pg_class
+        * statistics in this situation, since no new information has been
+        * provided.)
         */
        if (scanned_pages == 0)
                return old_rel_tuples;

diff --git a/src/backend/commands/vacuumlazy.c b/src/backend/commands/vacuumlazy.c
index 5c9c7bc3087255878a48d427089e01035d2d79c5..68909df1664d3b2ce56f898bd07c71755118c2d2 100644 (file)
--- a/src/backend/commands/vacuumlazy.c
+++ b/src/backend/commands/vacuumlazy.c
@@ -85,6 +85,7 @@ typedef struct LVRelStats
        /* hasindex = true means two-pass strategy; false means one-pass */
        bool            hasindex;
        /* Overall statistics about rel */
+       BlockNumber old_rel_pages;      /* previous value of pg_class.relpages */
        BlockNumber rel_pages;          /* total number of pages */
        BlockNumber scanned_pages;      /* number of pages we examined */
        double          scanned_tuples; /* counts only tuples on scanned pages */
@@ -157,6 +158,9 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
        TimestampTz starttime = 0;
        bool            scan_all;
        TransactionId freezeTableLimit;
+       BlockNumber new_rel_pages;
+       double          new_rel_tuples;
+       TransactionId new_frozen_xid;
        bool            heldoff = false;
 
        pg_rusage_init(&ru0);
@@ -180,6 +184,7 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
 
        vacrelstats = (LVRelStats *) palloc0(sizeof(LVRelStats));
 
+       vacrelstats->old_rel_pages = onerel->rd_rel->relpages;
        vacrelstats->old_rel_tuples = onerel->rd_rel->reltuples;
        vacrelstats->num_index_scans = 0;
 
@@ -219,21 +224,40 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
        FreeSpaceMapVacuum(onerel);
 
        /*
-        * Update statistics in pg_class.  But don't change relfrozenxid if we
-        * skipped any pages.
+        * Update statistics in pg_class.
+        *
+        * A corner case here is that if we scanned no pages at all because every
+        * page is all-visible, we should not update relpages/reltuples, because
+        * we have no new information to contribute.  In particular this keeps
+        * us from replacing relpages=reltuples=0 (which means "unknown tuple
+        * density") with nonzero relpages and reltuples=0 (which means "zero
+        * tuple density") unless there's some actual evidence for the latter.
+        *
+        * Also, don't change relfrozenxid if we skipped any pages, since then
+        * we don't know for certain that all tuples have a newer xmin.
         */
+       new_rel_pages = vacrelstats->rel_pages;
+       new_rel_tuples = vacrelstats->new_rel_tuples;
+       if (vacrelstats->scanned_pages == 0 && new_rel_pages > 0)
+       {
+               new_rel_pages = vacrelstats->old_rel_pages;
+               new_rel_tuples = vacrelstats->old_rel_tuples;
+       }
+
+       new_frozen_xid = FreezeLimit;
+       if (vacrelstats->scanned_pages < vacrelstats->rel_pages)
+               new_frozen_xid = InvalidTransactionId;
+
        vac_update_relstats(onerel,
-                                               vacrelstats->rel_pages, vacrelstats->new_rel_tuples,
+                                               new_rel_pages, new_rel_tuples,
                                                vacrelstats->hasindex,
-                                               (vacrelstats->scanned_pages < vacrelstats->rel_pages) ?
-                                               InvalidTransactionId :
-                                               FreezeLimit);
+                                               new_frozen_xid);
 
        /* report results to the stats collector, too */
        pgstat_report_vacuum(RelationGetRelid(onerel),
                                                 onerel->rd_rel->relisshared,
                                                 vacstmt->analyze,
-                                                vacrelstats->new_rel_tuples);
+                                                new_rel_tuples);
 
        /* and log the action if appropriate */
        if (IsAutoVacuumWorkerProcess() && Log_autovacuum_min_duration >= 0)
@@ -253,7 +277,7 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
                                                        vacrelstats->pages_removed,
                                                        vacrelstats->rel_pages,
                                                        vacrelstats->tuples_deleted,
-                                                       vacrelstats->new_rel_tuples,
+                                                       new_rel_tuples,
                                                        pg_rusage_show(&ru0))));
        }
 

diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 76dfa66e34376f49f845afcaa37355659b8ce4d7..66d33d86ac2d1d76a5197e3349dd7f4edef9099b 100644 (file)
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -1404,8 +1404,8 @@ formrdesc(const char *relationName, Oid relationReltype,
         */
        relation->rd_rel->relistemp = false;
 
-       relation->rd_rel->relpages = 1;
-       relation->rd_rel->reltuples = 1;
+       relation->rd_rel->relpages = 0;
+       relation->rd_rel->reltuples = 0;
        relation->rd_rel->relkind = RELKIND_RELATION;
        relation->rd_rel->relhasoids = hasoids;
        relation->rd_rel->relnatts = (int16) natts;