Re: [BUGS] Routine analyze of single column prevents standard autoanalyze from running at all - Mailing list pgsql-hackers

I wrote:
> Tomasz Ostrowski <tometzky+pg@ato.waw.pl> writes:
>> I suppose this is a bug - an analyze, which does not analyze all
>> columns, should not reset pg_stat_user_tables(n_mod_since_analyze). What
>> do you think?

> I'm inclined to think that this is a reasonable complaint.  A usage
> pattern like that probably hasn't come up before; but now that it has,
> it's clear it shouldn't block auto-analyze from happening.

Attached is a draft patch for this, which I propose to apply and
back-patch.  Any objections?

            regards, tom lane

diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index cf8c816..97059e5 100644
*** a/src/backend/commands/analyze.c
--- b/src/backend/commands/analyze.c
*************** do_analyze_rel(Relation onerel, int opti
*** 611,620 ****
      /*
       * Report ANALYZE to the stats collector, too.  However, if doing
       * inherited stats we shouldn't report, because the stats collector only
!      * tracks per-table stats.
       */
      if (!inh)
!         pgstat_report_analyze(onerel, totalrows, totaldeadrows);

      /* If this isn't part of VACUUM ANALYZE, let index AMs do cleanup */
      if (!(options & VACOPT_VACUUM))
--- 611,623 ----
      /*
       * Report ANALYZE to the stats collector, too.  However, if doing
       * inherited stats we shouldn't report, because the stats collector only
!      * tracks per-table stats.  Reset the changes_since_analyze counter only
!      * if we analyzed all columns; otherwise, there is still work for
!      * auto-analyze to do.
       */
      if (!inh)
!         pgstat_report_analyze(onerel, totalrows, totaldeadrows,
!                               (va_cols == NIL));

      /* If this isn't part of VACUUM ANALYZE, let index AMs do cleanup */
      if (!(options & VACOPT_VACUUM))
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 44237f9..439fc06 100644
*** a/src/backend/postmaster/pgstat.c
--- b/src/backend/postmaster/pgstat.c
*************** pgstat_report_vacuum(Oid tableoid, bool
*** 1340,1350 ****
   * pgstat_report_analyze() -
   *
   *    Tell the collector about the table we just analyzed.
   * --------
   */
  void
  pgstat_report_analyze(Relation rel,
!                       PgStat_Counter livetuples, PgStat_Counter deadtuples)
  {
      PgStat_MsgAnalyze msg;

--- 1340,1354 ----
   * pgstat_report_analyze() -
   *
   *    Tell the collector about the table we just analyzed.
+  *
+  * Caller must provide new live- and dead-tuples estimates, as well as a
+  * flag indicating whether to reset the changes_since_analyze counter.
   * --------
   */
  void
  pgstat_report_analyze(Relation rel,
!                       PgStat_Counter livetuples, PgStat_Counter deadtuples,
!                       bool resetcounter)
  {
      PgStat_MsgAnalyze msg;

*************** pgstat_report_analyze(Relation rel,
*** 1381,1386 ****
--- 1385,1391 ----
      msg.m_databaseid = rel->rd_rel->relisshared ? InvalidOid : MyDatabaseId;
      msg.m_tableoid = RelationGetRelid(rel);
      msg.m_autovacuum = IsAutoVacuumWorkerProcess();
+     msg.m_resetcounter = resetcounter;
      msg.m_analyzetime = GetCurrentTimestamp();
      msg.m_live_tuples = livetuples;
      msg.m_dead_tuples = deadtuples;
*************** pgstat_recv_analyze(PgStat_MsgAnalyze *m
*** 5263,5272 ****
      tabentry->n_dead_tuples = msg->m_dead_tuples;

      /*
!      * We reset changes_since_analyze to zero, forgetting any changes that
!      * occurred while the ANALYZE was in progress.
       */
!     tabentry->changes_since_analyze = 0;

      if (msg->m_autovacuum)
      {
--- 5268,5278 ----
      tabentry->n_dead_tuples = msg->m_dead_tuples;

      /*
!      * If commanded, reset changes_since_analyze to zero, forgetting any
!      * changes that occurred while the ANALYZE was in progress.
       */
!     if (msg->m_resetcounter)
!         tabentry->changes_since_analyze = 0;

      if (msg->m_autovacuum)
      {
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index a078b61..19c8387 100644
*** a/src/include/pgstat.h
--- b/src/include/pgstat.h
*************** typedef struct PgStat_MsgAnalyze
*** 383,388 ****
--- 383,389 ----
      Oid            m_databaseid;
      Oid            m_tableoid;
      bool        m_autovacuum;
+     bool        m_resetcounter;
      TimestampTz m_analyzetime;
      PgStat_Counter m_live_tuples;
      PgStat_Counter m_dead_tuples;
*************** extern void pgstat_report_autovac(Oid db
*** 970,976 ****
  extern void pgstat_report_vacuum(Oid tableoid, bool shared,
                       PgStat_Counter livetuples, PgStat_Counter deadtuples);
  extern void pgstat_report_analyze(Relation rel,
!                       PgStat_Counter livetuples, PgStat_Counter deadtuples);

  extern void pgstat_report_recovery_conflict(int reason);
  extern void pgstat_report_deadlock(void);
--- 971,978 ----
  extern void pgstat_report_vacuum(Oid tableoid, bool shared,
                       PgStat_Counter livetuples, PgStat_Counter deadtuples);
  extern void pgstat_report_analyze(Relation rel,
!                       PgStat_Counter livetuples, PgStat_Counter deadtuples,
!                       bool resetcounter);

  extern void pgstat_report_recovery_conflict(int reason);
  extern void pgstat_report_deadlock(void);