Thread: Re: Row estimates for empty tables

[ redirecting to -hackers ]

I wrote:
> The core issue here is "how do we know whether the table is likely to stay
> empty?".  I can think of a couple of more or less klugy solutions:

> 1. Arrange to send out a relcache inval when adding the first page to
> a table, and then remove the planner hack for disbelieving relpages = 0.
> I fear this'd be a mess from a system structural standpoint, but it might
> work fairly transparently.

I experimented with doing this.  It's not hard to code, if you don't mind
having RelationGetBufferForTuple calling CacheInvalidateRelcache.  I'm not
sure whether that code path might cause any long-term problems, but it
seems to work OK right now.  However, this solution causes massive
"failures" in the regression tests as a result of plans changing.  I'm
sure that's partly because we use so many small tables in the tests.
Nonetheless, it's not promising from the standpoint of not causing
unexpected problems in the real world.

> 2. Establish the convention that vacuuming or analyzing an empty table
> is what you do to tell the system that this state is going to persist.
> That's more or less what the existing comments in plancat.c envision,
> but we never made a definition for how the occurrence of that event
> would be recorded in the catalogs, other than setting relpages > 0.
> Rather than adding another pg_class column, I'm tempted to say that
> vacuum/analyze should set relpages to a minimum of 1, even if the
> relation has zero pages.

I also tried this, and it seems a lot more promising: no existing
regression test cases change.  So perhaps we should do the attached
or something like it.

            regards, tom lane

diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 1bbc4598f7..243cdaa409 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -594,6 +594,19 @@ heap_vacuum_rel(Relation onerel, VacuumParams *params,
     new_frozen_xid = scanned_all_unfrozen ? FreezeLimit : InvalidTransactionId;
     new_min_multi = scanned_all_unfrozen ? MultiXactCutoff : InvalidMultiXactId;

+    /*
+     * Another corner case is that if the relation is physically zero-length,
+     * we don't want to record relpages=reltuples=0 in pg_class; what we want
+     * to record is relpages=1, reltuples=0.  This tells the planner that the
+     * relation has been seen to be empty by VACUUM or ANALYZE, so it should
+     * not override a small measured table size.
+     */
+    if (new_rel_pages == 0)
+    {
+        Assert(new_live_tuples == 0);
+        new_rel_pages = 1;
+    }
+
     vac_update_relstats(onerel,
                         new_rel_pages,
                         new_live_tuples,
diff --git a/src/backend/access/table/tableam.c b/src/backend/access/table/tableam.c
index 4b2bb29559..0a92450e8a 100644
--- a/src/backend/access/table/tableam.c
+++ b/src/backend/access/table/tableam.c
@@ -585,11 +585,9 @@ table_block_relation_estimate_size(Relation rel, int32 *attr_widths,
      * doesn't happen instantaneously, and it won't happen at all for cases
      * such as temporary tables.)
      *
-     * We approximate "never vacuumed" by "has relpages = 0", which means this
-     * will also fire on genuinely empty relations.  Not great, but
-     * fortunately that's a seldom-seen case in the real world, and it
-     * shouldn't degrade the quality of the plan too much anyway to err in
-     * this direction.
+     * We test "never vacuumed" by seeing whether relpages = 0.  VACUUM and
+     * ANALYZE will never set relpages to less than 1, even if the relation is
+     * in fact zero length.
      *
      * If the table has inheritance children, we don't apply this heuristic.
      * Totally empty parent tables are quite common, so we should be willing
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index 924ef37c81..f0247e350f 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -620,6 +620,16 @@ do_analyze_rel(Relation onerel, VacuumParams *params,

         visibilitymap_count(onerel, &relallvisible, NULL);

+        /*
+         * As in VACUUM, replace relpages=reltuples=0 with relpages=1,
+         * reltuples=0.
+         */
+        if (relpages == 0)
+        {
+            Assert(totalrows == 0);
+            relpages = 1;
+        }
+
         vac_update_relstats(onerel,
                             relpages,
                             totalrows,

On Sat, 25 Jul 2020 at 10:34, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I wrote:
> > 1. Arrange to send out a relcache inval when adding the first page to
> > a table, and then remove the planner hack for disbelieving relpages = 0.
> > I fear this'd be a mess from a system structural standpoint, but it might
> > work fairly transparently.
>
> I experimented with doing this.  It's not hard to code, if you don't mind
> having RelationGetBufferForTuple calling CacheInvalidateRelcache.  I'm not
> sure whether that code path might cause any long-term problems, but it
> seems to work OK right now.  However, this solution causes massive
> "failures" in the regression tests as a result of plans changing.  I'm
> sure that's partly because we use so many small tables in the tests.
> Nonetheless, it's not promising from the standpoint of not causing
> unexpected problems in the real world.

I guess all these changes would be the planner moving towards a plan
that suits having fewer rows for the given table better.  If so, that
does seem quite scary as we already have enough problems from the
planner choosing poor plans when it thinks there are fewer rows than
there actually are.  Don't we need to keep something like the 10-page
estimate there so safer plans are produced before auto-vacuum gets in
and gathers some proper stats?

I think if anything we'd want to move in the direction of producing
more cautious plans when the estimated number of rows is low. Perhaps
especially so for when the planner opts to do things like perform a
non-parameterized nested loop join when it thinks the RelOptInfo with,
say 3, unbeknown-to-the-planner, correlated, base restrict quals that
are thought to produce just 1 row, but actually produce many more.

> > 2. Establish the convention that vacuuming or analyzing an empty table
> > is what you do to tell the system that this state is going to persist.
> > That's more or less what the existing comments in plancat.c envision,
> > but we never made a definition for how the occurrence of that event
> > would be recorded in the catalogs, other than setting relpages > 0.
> > Rather than adding another pg_class column, I'm tempted to say that
> > vacuum/analyze should set relpages to a minimum of 1, even if the
> > relation has zero pages.
>
> I also tried this, and it seems a lot more promising: no existing
> regression test cases change.  So perhaps we should do the attached
> or something like it.

This sounds like a more plausible solution. At least this way there's
an escape hatch for people who suffer due to this.

David

Pavel Stehule <pavel.stehule@gmail.com> writes:
> I am sending a patch that is years used in GoodData.

I'm quite unexcited about that.  I'd be the first to agree that the
ten-pages estimate is a hack, but it's not an improvement to ask users
to think of a better value ... especially not as a one-size-fits-
all-relations GUC setting.

I did have an idea that I think is better than my previous one:
rather than lying about the value of relpages, let's represent the
case where we don't know the tuple density by setting reltuples = -1
initially.  This leads to a patch that's a good bit more invasive than
the quick-hack solution, but I think it's a lot cleaner on the whole.

A possible objection is that this changes the FDW API slightly, as
GetForeignRelSize callbacks now need to deal with rel->tuples possibly
being -1.  We could avoid an API break if we made plancat.c clamp
that value to zero; but then FDWs still couldn't tell the difference
between the "empty" and "never analyzed" cases, and I think this is
just as much of an issue for them as for the core code.

I'll add this to the upcoming CF.

            regards, tom lane

diff --git a/contrib/file_fdw/file_fdw.c b/contrib/file_fdw/file_fdw.c
index fbcf7ca9c9..072a6dc1c1 100644
--- a/contrib/file_fdw/file_fdw.c
+++ b/contrib/file_fdw/file_fdw.c
@@ -996,7 +996,7 @@ estimate_size(PlannerInfo *root, RelOptInfo *baserel,
     /*
      * Estimate the number of tuples in the file.
      */
-    if (baserel->pages > 0)
+    if (baserel->tuples >= 0 && baserel->pages > 0)
     {
         /*
          * We have # of pages and # of tuples from pg_class (that is, from a
diff --git a/contrib/pgstattuple/pgstatapprox.c b/contrib/pgstattuple/pgstatapprox.c
index 3a99333d44..23306e11a7 100644
--- a/contrib/pgstattuple/pgstatapprox.c
+++ b/contrib/pgstattuple/pgstatapprox.c
@@ -195,6 +195,9 @@ statapprox_heap(Relation rel, output_type *stat)
     stat->tuple_count = vac_estimate_reltuples(rel, nblocks, scanned,
                                                stat->tuple_count);

+    /* It's not clear if we could get -1 here, but be safe. */
+    stat->tuple_count = Max(stat->tuple_count, 0);
+
     /*
      * Calculate percentages if the relation has one or more pages.
      */
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 9fc53cad68..a31abce7c9 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -692,15 +692,14 @@ postgresGetForeignRelSize(PlannerInfo *root,
     else
     {
         /*
-         * If the foreign table has never been ANALYZEd, it will have relpages
-         * and reltuples equal to zero, which most likely has nothing to do
-         * with reality.  We can't do a whole lot about that if we're not
+         * If the foreign table has never been ANALYZEd, it will have
+         * reltuples < 0, meaning "unknown".  We can't do much if we're not
          * allowed to consult the remote server, but we can use a hack similar
          * to plancat.c's treatment of empty relations: use a minimum size
          * estimate of 10 pages, and divide by the column-datatype-based width
          * estimate to get the corresponding number of tuples.
          */
-        if (baserel->pages == 0 && baserel->tuples == 0)
+        if (baserel->tuples < 0)
         {
             baserel->pages = 10;
             baserel->tuples =
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 1232b24e74..7861379d97 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -1977,6 +1977,10 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l
        the planner.  It is updated by <command>VACUUM</command>,
        <command>ANALYZE</command>, and a few DDL commands such as
        <command>CREATE INDEX</command>.
+       If the table has never yet been vacuumed or
+       analyzed, <structfield>reltuples</structfield>
+       contains <literal>-1</literal> indicating that the row count is
+       unknown.
       </para></entry>
      </row>

diff --git a/doc/src/sgml/fdwhandler.sgml b/doc/src/sgml/fdwhandler.sgml
index 74793035d7..72fa127212 100644
--- a/doc/src/sgml/fdwhandler.sgml
+++ b/doc/src/sgml/fdwhandler.sgml
@@ -130,7 +130,8 @@ GetForeignRelSize(PlannerInfo *root,
      (The initial value is
      from <structname>pg_class</structname>.<structfield>reltuples</structfield>
      which represents the total row count seen by the
-     last <command>ANALYZE</command>.)
+     last <command>ANALYZE</command>; it will be <literal>-1</literal> if
+     no <command>ANALYZE</command> has been done on this foreign table.)
     </para>

     <para>
diff --git a/src/backend/access/gin/ginvacuum.c b/src/backend/access/gin/ginvacuum.c
index 9cd6638df6..0935a6d9e5 100644
--- a/src/backend/access/gin/ginvacuum.c
+++ b/src/backend/access/gin/ginvacuum.c
@@ -727,7 +727,7 @@ ginvacuumcleanup(IndexVacuumInfo *info, IndexBulkDeleteResult *stats)
      * entries.  This is bogus if the index is partial, but it's real hard to
      * tell how many distinct heap entries are referenced by a GIN index.
      */
-    stats->num_index_tuples = info->num_heap_tuples;
+    stats->num_index_tuples = Max(info->num_heap_tuples, 0);
     stats->estimated_count = info->estimated_count;

     /*
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 44e2224dd5..52dd0c59f0 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -208,7 +208,8 @@ typedef struct LVShared
      * live tuples in the index vacuum case or the new live tuples in the
      * index cleanup case.
      *
-     * estimated_count is true if reltuples is an estimated value.
+     * estimated_count is true if reltuples is an estimated value.  (Note that
+     * reltuples could be -1 in this case, indicating we have no idea.)
      */
     double        reltuples;
     bool        estimated_count;
@@ -561,31 +562,19 @@ heap_vacuum_rel(Relation onerel, VacuumParams *params,
     /*
      * 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.
+     * In principle new_live_tuples could be -1 indicating that we (still)
+     * don't know the tuple count.  In practice that probably can't happen,
+     * since we'd surely have scanned some pages if the table is new and
+     * nonempty.
      *
-     * It's important that we use tupcount_pages and not scanned_pages for the
-     * check described above; scanned_pages counts pages where we could not
-     * get cleanup lock, and which were processed only for frozenxid purposes.
-     *
-     * We do update relallvisible even in the corner case, since if the table
-     * is all-visible we'd definitely like to know that.  But clamp the value
-     * to be not more than what we're setting relpages to.
+     * For safety, clamp relallvisible to be not more than what we're setting
+     * relpages to.
      *
      * Also, don't change relfrozenxid/relminmxid 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_live_tuples = vacrelstats->new_live_tuples;
-    if (vacrelstats->tupcount_pages == 0 && new_rel_pages > 0)
-    {
-        new_rel_pages = vacrelstats->old_rel_pages;
-        new_live_tuples = vacrelstats->old_live_tuples;
-    }

     visibilitymap_count(onerel, &new_rel_allvisible, NULL);
     if (new_rel_allvisible > new_rel_pages)
@@ -606,7 +595,7 @@ heap_vacuum_rel(Relation onerel, VacuumParams *params,
     /* report results to the stats collector, too */
     pgstat_report_vacuum(RelationGetRelid(onerel),
                          onerel->rd_rel->relisshared,
-                         new_live_tuples,
+                         Max(new_live_tuples, 0),
                          vacrelstats->new_dead_tuples);
     pgstat_progress_end_command();

@@ -1674,9 +1663,12 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
                                                           vacrelstats->tupcount_pages,
                                                           live_tuples);

-    /* also compute total number of surviving heap entries */
+    /*
+     * Also compute the total number of surviving heap entries.  In the
+     * (unlikely) scenario that new_live_tuples is -1, take it as zero.
+     */
     vacrelstats->new_rel_tuples =
-        vacrelstats->new_live_tuples + vacrelstats->new_dead_tuples;
+        Max(vacrelstats->new_live_tuples, 0) + vacrelstats->new_dead_tuples;

     /*
      * Release any remaining pin on visibility map page.
@@ -2401,7 +2393,7 @@ lazy_cleanup_all_indexes(Relation *Irel, IndexBulkDeleteResult **stats,
  *        dead_tuples, and update running statistics.
  *
  *        reltuples is the number of heap tuples to be passed to the
- *        bulkdelete callback.
+ *        bulkdelete callback.  It's always assumed to be estimated.
  */
 static void
 lazy_vacuum_index(Relation indrel, IndexBulkDeleteResult **stats,
diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index 8fa6ac7296..c822b49a71 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -853,6 +853,7 @@ _bt_vacuum_needs_cleanup(IndexVacuumInfo *info)
         prev_num_heap_tuples = metad->btm_last_cleanup_num_heap_tuples;

         if (cleanup_scale_factor <= 0 ||
+            info->num_heap_tuples < 0 ||
             prev_num_heap_tuples <= 0 ||
             (info->num_heap_tuples - prev_num_heap_tuples) /
             prev_num_heap_tuples >= cleanup_scale_factor)
diff --git a/src/backend/access/table/tableam.c b/src/backend/access/table/tableam.c
index c638319765..6438c45716 100644
--- a/src/backend/access/table/tableam.c
+++ b/src/backend/access/table/tableam.c
@@ -701,18 +701,14 @@ table_block_relation_estimate_size(Relation rel, int32 *attr_widths,
      * doesn't happen instantaneously, and it won't happen at all for cases
      * such as temporary tables.)
      *
-     * We approximate "never vacuumed" by "has relpages = 0", which means this
-     * will also fire on genuinely empty relations.  Not great, but
-     * fortunately that's a seldom-seen case in the real world, and it
-     * shouldn't degrade the quality of the plan too much anyway to err in
-     * this direction.
+     * We test "never vacuumed" by seeing whether reltuples < 0.
      *
      * If the table has inheritance children, we don't apply this heuristic.
      * Totally empty parent tables are quite common, so we should be willing
      * to believe that they are empty.
      */
     if (curpages < 10 &&
-        relpages == 0 &&
+        reltuples < 0 &&
         !rel->rd_rel->relhassubclass)
         curpages = 10;

@@ -727,17 +723,17 @@ table_block_relation_estimate_size(Relation rel, int32 *attr_widths,
     }

     /* estimate number of tuples from previous tuple density */
-    if (relpages > 0)
+    if (reltuples >= 0 && relpages > 0)
         density = reltuples / (double) relpages;
     else
     {
         /*
-         * When we have no data because the relation was truncated, estimate
-         * tuple width from attribute datatypes.  We assume here that the
-         * pages are completely full, which is OK for tables (since they've
-         * presumably not been VACUUMed yet) but is probably an overestimate
-         * for indexes.  Fortunately get_relation_info() can clamp the
-         * overestimate to the parent table's size.
+         * When we have no data because the relation was never yet vacuumed,
+         * estimate tuple width from attribute datatypes.  We assume here that
+         * the pages are completely full, which is OK for tables but is
+         * probably an overestimate for indexes.  Fortunately
+         * get_relation_info() can clamp the overestimate to the parent
+         * table's size.
          *
          * Note: this code intentionally disregards alignment considerations,
          * because (a) that would be gilding the lily considering how crude
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index f2ca686397..abd5bdb866 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -1015,7 +1015,7 @@ AddNewRelationTuple(Relation pg_class_desc,
         case RELKIND_TOASTVALUE:
             /* The relation is real, but as yet empty */
             new_rel_reltup->relpages = 0;
-            new_rel_reltup->reltuples = 0;
+            new_rel_reltup->reltuples = -1;
             new_rel_reltup->relallvisible = 0;
             break;
         case RELKIND_SEQUENCE:
@@ -1027,7 +1027,7 @@ AddNewRelationTuple(Relation pg_class_desc,
         default:
             /* Views, etc, have no disk storage */
             new_rel_reltup->relpages = 0;
-            new_rel_reltup->reltuples = 0;
+            new_rel_reltup->reltuples = -1;
             new_rel_reltup->relallvisible = 0;
             break;
     }
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 1be27eec52..daeac6b5a5 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -2722,6 +2722,15 @@ index_update_stats(Relation rel,
     /* Should this be a more comprehensive test? */
     Assert(rd_rel->relkind != RELKIND_PARTITIONED_INDEX);

+    /*
+     * As a special hack, if we are dealing with an empty table and the
+     * existing reltuples is -1, we leave that alone.  This ensures that
+     * creating an index as part of CREATE TABLE doesn't cause the table to
+     * prematurely look like it's been vacuumed.
+     */
+    if (reltuples == 0 && rd_rel->reltuples < 0)
+        reltuples = -1;
+
     /* Apply required updates, if any, to copied tuple */

     dirty = false;
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 23eb605d4c..308a51d95d 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -1128,8 +1128,8 @@ vacuum_set_xid_limits(Relation rel,
  *        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.
+ *        the old value of pg_class.reltuples/pg_class.relpages as a measurement
+ *        of the tuple density in the unscanned pages.
  *
  *        Note: scanned_tuples should count only *live* tuples, since
  *        pg_class.reltuples is defined that way.
@@ -1152,18 +1152,16 @@ vac_estimate_reltuples(Relation relation,

     /*
      * 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.)
+     * of reltuples.  (Note: we might be returning -1 in this case.)
      */
     if (scanned_pages == 0)
         return old_rel_tuples;

     /*
-     * If old value of relpages is zero, old density is indeterminate; we
-     * can't do much except scale up scanned_tuples to match total_pages.
+     * If old density is unknown, we can't do much except scale up
+     * scanned_tuples to match total_pages.
      */
-    if (old_rel_pages == 0)
+    if (old_rel_tuples < 0 || old_rel_pages == 0)
         return floor((scanned_tuples / scanned_pages) * total_pages + 0.5);

     /*
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index 0eeff804bc..b399592ff8 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -912,7 +912,11 @@ set_foreign_size(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte)
     /* ... but do not let it set the rows estimate to zero */
     rel->rows = clamp_row_est(rel->rows);

-    /* also, make sure rel->tuples is not insane relative to rel->rows */
+    /*
+     * Also, make sure rel->tuples is not insane relative to rel->rows.
+     * Notably, this ensures sanity if pg_class.reltuples contains -1 and the
+     * FDW doesn't do anything to replace that.
+     */
     rel->tuples = Max(rel->tuples, rel->rows);
 }

diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index 25545029d7..f9d0d67aa7 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -974,11 +974,6 @@ estimate_rel_size(Relation rel, int32 *attr_widths,
             /* it has storage, ok to call the smgr */
             curpages = RelationGetNumberOfBlocks(rel);

-            /* coerce values in pg_class to more desirable types */
-            relpages = (BlockNumber) rel->rd_rel->relpages;
-            reltuples = (double) rel->rd_rel->reltuples;
-            relallvisible = (BlockNumber) rel->rd_rel->relallvisible;
-
             /* report estimated # pages */
             *pages = curpages;
             /* quick exit if rel is clearly empty */
@@ -988,6 +983,7 @@ estimate_rel_size(Relation rel, int32 *attr_widths,
                 *allvisfrac = 0;
                 break;
             }
+
             /* coerce values in pg_class to more desirable types */
             relpages = (BlockNumber) rel->rd_rel->relpages;
             reltuples = (double) rel->rd_rel->reltuples;
@@ -1006,12 +1002,12 @@ estimate_rel_size(Relation rel, int32 *attr_widths,
             }

             /* estimate number of tuples from previous tuple density */
-            if (relpages > 0)
+            if (reltuples >= 0 && relpages > 0)
                 density = reltuples / (double) relpages;
             else
             {
                 /*
-                 * When we have no data because the relation was truncated,
+                 * If we have no data because the relation was never vacuumed,
                  * estimate tuple width from attribute datatypes.  We assume
                  * here that the pages are completely full, which is OK for
                  * tables (since they've presumably not been VACUUMed yet) but
@@ -1059,6 +1055,7 @@ estimate_rel_size(Relation rel, int32 *attr_widths,
             break;
         case RELKIND_FOREIGN_TABLE:
             /* Just use whatever's in pg_class */
+            /* Note that FDW must cope if reltuples is -1! */
             *pages = rel->rd_rel->relpages;
             *tuples = rel->rd_rel->reltuples;
             *allvisfrac = 0;
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index c6ec657a93..1b8cd7bacd 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -3080,6 +3080,10 @@ relation_needs_vacanalyze(Oid relid,
         instuples = tabentry->inserts_since_vacuum;
         anltuples = tabentry->changes_since_analyze;

+        /* If the table hasn't yet been vacuumed, take reltuples as zero */
+        if (reltuples < 0)
+            reltuples = 0;
+
         vacthresh = (float4) vac_base_thresh + vac_scale_factor * reltuples;
         vacinsthresh = (float4) vac_ins_base_thresh + vac_ins_scale_factor * reltuples;
         anlthresh = (float4) anl_base_thresh + anl_scale_factor * reltuples;
diff --git a/src/backend/rewrite/rewriteDefine.c b/src/backend/rewrite/rewriteDefine.c
index 9989df1107..8ef0917021 100644
--- a/src/backend/rewrite/rewriteDefine.c
+++ b/src/backend/rewrite/rewriteDefine.c
@@ -621,7 +621,7 @@ DefineQueryRewrite(const char *rulename,
         classForm->relam = InvalidOid;
         classForm->reltablespace = InvalidOid;
         classForm->relpages = 0;
-        classForm->reltuples = 0;
+        classForm->reltuples = -1;
         classForm->relallvisible = 0;
         classForm->reltoastrelid = InvalidOid;
         classForm->relhasindex = false;
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index a2453cf1f4..96ecad02dd 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -1870,7 +1870,7 @@ formrdesc(const char *relationName, Oid relationReltype,

     relation->rd_rel->relreplident = REPLICA_IDENTITY_NOTHING;
     relation->rd_rel->relpages = 0;
-    relation->rd_rel->reltuples = 0;
+    relation->rd_rel->reltuples = -1;
     relation->rd_rel->relallvisible = 0;
     relation->rd_rel->relkind = RELKIND_RELATION;
     relation->rd_rel->relnatts = (int16) natts;
@@ -3692,7 +3692,7 @@ RelationSetNewRelfilenode(Relation relation, char persistence)
         if (relation->rd_rel->relkind != RELKIND_SEQUENCE)
         {
             classform->relpages = 0;    /* it's empty until further notice */
-            classform->reltuples = 0;
+            classform->reltuples = -1;
             classform->relallvisible = 0;
         }
         classform->relfrozenxid = freezeXid;
diff --git a/src/include/access/genam.h b/src/include/access/genam.h
index 931257bd81..68d90f5141 100644
--- a/src/include/access/genam.h
+++ b/src/include/access/genam.h
@@ -38,8 +38,8 @@ typedef struct IndexBuildResult
  *
  * num_heap_tuples is accurate only when estimated_count is false;
  * otherwise it's just an estimate (currently, the estimate is the
- * prior value of the relation's pg_class.reltuples field).  It will
- * always just be an estimate during ambulkdelete.
+ * prior value of the relation's pg_class.reltuples field, so it could
+ * even be -1).  It will always just be an estimate during ambulkdelete.
  */
 typedef struct IndexVacuumInfo
 {
diff --git a/src/include/catalog/pg_class.h b/src/include/catalog/pg_class.h
index 78b33b2a7f..679eec3443 100644
--- a/src/include/catalog/pg_class.h
+++ b/src/include/catalog/pg_class.h
@@ -62,8 +62,8 @@ CATALOG(pg_class,1259,RelationRelationId) BKI_BOOTSTRAP BKI_ROWTYPE_OID(83,Relat
     /* # of blocks (not always up-to-date) */
     int32        relpages BKI_DEFAULT(0);

-    /* # of tuples (not always up-to-date) */
-    float4        reltuples BKI_DEFAULT(0);
+    /* # of tuples (not always up-to-date; -1 means "unknown") */
+    float4        reltuples BKI_DEFAULT(-1);

     /* # of all-visible blocks (not always up-to-date) */
     int32        relallvisible BKI_DEFAULT(0);

Pavel Stehule <pavel.stehule@gmail.com> writes:
> I'll mark this patch as ready for commit

Pushed, thanks for looking.

            regards, tom lane