Re: [patch] BUG #15005: ANALYZE can make pg_class.reltuples inaccurate.

I wrote:
> Maybe this type of situation is an argument for trusting an ANALYZE-based
> estimate more than the VACUUM-based estimate.  I remain uncomfortable with
> that in cases where VACUUM looked at much more of the table than ANALYZE
> did, though.  Maybe we need some heuristic based on the number of pages
> actually visited by each pass?

I looked into doing something like that.  It's possible, but it's fairly
invasive; there's no clean way to compare those page counts without
altering the API of acquire_sample_rows() to pass back the number of pages
it visited.  That would mean a change in FDW-visible APIs.  We could do
that, but I don't want to insist on it when there's nothing backing it up
except a fear that *maybe* ANALYZE's estimate will be wrong often enough
to worry about.

So at this point I'm prepared to go forward with your patch, though not
to risk back-patching it.  Field experience will tell us if we need to
do more.  I propose the attached cosmetic refactoring, though.

            regards, tom lane

diff --git a/contrib/pgstattuple/pgstatapprox.c b/contrib/pgstattuple/pgstatapprox.c
index 3cfbc08..474c3bd 100644
*** a/contrib/pgstattuple/pgstatapprox.c
--- b/contrib/pgstattuple/pgstatapprox.c
*************** statapprox_heap(Relation rel, output_typ
*** 184,190 ****

      stat->table_len = (uint64) nblocks * BLCKSZ;

!     stat->tuple_count = vac_estimate_reltuples(rel, false, nblocks, scanned,
                                                 stat->tuple_count + misc_count);

      /*
--- 184,190 ----

      stat->table_len = (uint64) nblocks * BLCKSZ;

!     stat->tuple_count = vac_estimate_reltuples(rel, nblocks, scanned,
                                                 stat->tuple_count + misc_count);

      /*
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index 5f21fcb..ef93fb4 100644
*** a/src/backend/commands/analyze.c
--- b/src/backend/commands/analyze.c
*************** acquire_sample_rows(Relation onerel, int
*** 1249,1267 ****
          qsort((void *) rows, numrows, sizeof(HeapTuple), compare_rows);

      /*
!      * Estimate total numbers of rows in relation.  For live rows, use
!      * vac_estimate_reltuples; for dead rows, we have no source of old
!      * information, so we have to assume the density is the same in unseen
!      * pages as in the pages we scanned.
       */
-     *totalrows = vac_estimate_reltuples(onerel, true,
-                                         totalblocks,
-                                         bs.m,
-                                         liverows);
      if (bs.m > 0)
          *totaldeadrows = floor((deadrows / bs.m) * totalblocks + 0.5);
      else
          *totaldeadrows = 0.0;

      /*
       * Emit some interesting relation info
--- 1249,1270 ----
          qsort((void *) rows, numrows, sizeof(HeapTuple), compare_rows);

      /*
!      * Estimate total numbers of live and dead rows in relation, extrapolating
!      * on the assumption that the average tuple density in pages we didn't
!      * scan is the same as in the pages we did scan.  Since what we scanned is
!      * a random sample of the pages in the relation, this should be a good
!      * assumption.
       */
      if (bs.m > 0)
+     {
+         *totalrows = floor((liverows / bs.m) * totalblocks + 0.5);
          *totaldeadrows = floor((deadrows / bs.m) * totalblocks + 0.5);
+     }
      else
+     {
+         *totalrows = 0.0;
          *totaldeadrows = 0.0;
+     }

      /*
       * Emit some interesting relation info
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 7aca69a..b50c554 100644
*** a/src/backend/commands/vacuum.c
--- b/src/backend/commands/vacuum.c
*************** vacuum_set_xid_limits(Relation rel,
*** 766,781 ****
   * vac_estimate_reltuples() -- estimate the new value for pg_class.reltuples
   *
   *        If we scanned the whole relation then we should just use the count of
!  *        live tuples seen; but if we did not, we should not trust the count
!  *        unreservedly, especially not in VACUUM, which may have scanned a quite
!  *        nonrandom subset of the table.  When we have only partial information,
!  *        we take the old value of pg_class.reltuples as a measurement of the
   *        tuple density in the unscanned pages.
-  *
-  *        This routine is shared by VACUUM and ANALYZE.
   */
  double
! vac_estimate_reltuples(Relation relation, bool is_analyze,
                         BlockNumber total_pages,
                         BlockNumber scanned_pages,
                         double scanned_tuples)
--- 766,779 ----
   * vac_estimate_reltuples() -- estimate the new value for pg_class.reltuples
   *
   *        If we scanned the whole relation then we should just use the count of
!  *        live tuples seen; but if we did not, we should not blindly extrapolate
!  *        from that number, since VACUUM may have scanned a quite nonrandom
!  *        subset of the table.  When we have only partial information, we take
!  *        the old value of pg_class.reltuples as a measurement of the
   *        tuple density in the unscanned pages.
   */
  double
! vac_estimate_reltuples(Relation relation,
                         BlockNumber total_pages,
                         BlockNumber scanned_pages,
                         double scanned_tuples)
*************** vac_estimate_reltuples(Relation relation
*** 783,791 ****
      BlockNumber old_rel_pages = relation->rd_rel->relpages;
      double        old_rel_tuples = relation->rd_rel->reltuples;
      double        old_density;
!     double        new_density;
!     double        multiplier;
!     double        updated_density;

      /* If we did scan the whole table, just use the count as-is */
      if (scanned_pages >= total_pages)
--- 781,788 ----
      BlockNumber old_rel_pages = relation->rd_rel->relpages;
      double        old_rel_tuples = relation->rd_rel->reltuples;
      double        old_density;
!     double        unscanned_pages;
!     double        total_tuples;

      /* If we did scan the whole table, just use the count as-is */
      if (scanned_pages >= total_pages)
*************** vac_estimate_reltuples(Relation relation
*** 809,839 ****

      /*
       * Okay, we've covered the corner cases.  The normal calculation is to
!      * convert the old measurement to a density (tuples per page), then update
!      * the density using an exponential-moving-average approach, and finally
!      * compute reltuples as updated_density * total_pages.
!      *
!      * For ANALYZE, the moving average multiplier is just the fraction of the
!      * table's pages we scanned.  This is equivalent to assuming that the
!      * tuple density in the unscanned pages didn't change.  Of course, it
!      * probably did, if the new density measurement is different. But over
!      * repeated cycles, the value of reltuples will converge towards the
!      * correct value, if repeated measurements show the same new density.
!      *
!      * For VACUUM, the situation is a bit different: we have looked at a
!      * nonrandom sample of pages, but we know for certain that the pages we
!      * didn't look at are precisely the ones that haven't changed lately.
!      * Thus, there is a reasonable argument for doing exactly the same thing
!      * as for the ANALYZE case, that is use the old density measurement as the
!      * value for the unscanned pages.
!      *
!      * This logic could probably use further refinement.
       */
      old_density = old_rel_tuples / old_rel_pages;
!     new_density = scanned_tuples / scanned_pages;
!     multiplier = (double) scanned_pages / (double) total_pages;
!     updated_density = old_density + (new_density - old_density) * multiplier;
!     return floor(updated_density * total_pages + 0.5);
  }


--- 806,819 ----

      /*
       * Okay, we've covered the corner cases.  The normal calculation is to
!      * convert the old measurement to a density (tuples per page), then
!      * estimate the number of tuples in the unscanned pages using that figure,
!      * and finally add on the number of tuples in the scanned pages.
       */
      old_density = old_rel_tuples / old_rel_pages;
!     unscanned_pages = (double) total_pages - (double) scanned_pages;
!     total_tuples = old_density * unscanned_pages + scanned_tuples;
!     return floor(total_tuples + 0.5);
  }


diff --git a/src/backend/commands/vacuumlazy.c b/src/backend/commands/vacuumlazy.c
index cf7f5e1..9ac84e8 100644
*** a/src/backend/commands/vacuumlazy.c
--- b/src/backend/commands/vacuumlazy.c
*************** lazy_scan_heap(Relation onerel, int opti
*** 1286,1292 ****
      vacrelstats->new_dead_tuples = nkeep;

      /* now we can compute the new value for pg_class.reltuples */
!     vacrelstats->new_rel_tuples = vac_estimate_reltuples(onerel, false,
                                                           nblocks,
                                                           vacrelstats->tupcount_pages,
                                                           num_tuples);
--- 1286,1292 ----
      vacrelstats->new_dead_tuples = nkeep;

      /* now we can compute the new value for pg_class.reltuples */
!     vacrelstats->new_rel_tuples = vac_estimate_reltuples(onerel,
                                                           nblocks,
                                                           vacrelstats->tupcount_pages,
                                                           num_tuples);
diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
index 797b6df..85d472f 100644
*** a/src/include/commands/vacuum.h
--- b/src/include/commands/vacuum.h
*************** extern void vacuum(int options, List *re
*** 162,168 ****
  extern void vac_open_indexes(Relation relation, LOCKMODE lockmode,
                   int *nindexes, Relation **Irel);
  extern void vac_close_indexes(int nindexes, Relation *Irel, LOCKMODE lockmode);
! extern double vac_estimate_reltuples(Relation relation, bool is_analyze,
                         BlockNumber total_pages,
                         BlockNumber scanned_pages,
                         double scanned_tuples);
--- 162,168 ----
  extern void vac_open_indexes(Relation relation, LOCKMODE lockmode,
                   int *nindexes, Relation **Irel);
  extern void vac_close_indexes(int nindexes, Relation *Irel, LOCKMODE lockmode);
! extern double vac_estimate_reltuples(Relation relation,
                         BlockNumber total_pages,
                         BlockNumber scanned_pages,
                         double scanned_tuples);