On Mon, Aug 16, 2021 at 05:28:10PM -0500, Justin Pryzby wrote: > On Mon, Aug 16, 2021 at 05:42:48PM -0400, Álvaro Herrera wrote: > > On 2021-Aug-16, Álvaro Herrera wrote: > > > > > Here's the reversal patch for the 14 branch. (It applies cleanly to > > > master, but the unused member of PgStat_StatTabEntry needs to be > > > removed and catversion bumped). > > > > I have pushed this to both branches. (I did not remove the item from > > the release notes in the 14 branch.) > > | I retained the addition of relkind 'p' to tables included by > | pg_stat_user_tables, because reverting that would require a catversion > | bump. > > Right now, on v15dev, it shows 0, which is misleading. > Shouldn't it be null ? > > analyze_count | 0 > > Note that having analyze_count and last_analyze would be an an independently > useful change. Since parent tables aren't analyzed automatically, I have a > script to periodically process them if they weren't processed recently. Right > now, for partitioned tables, the best I could find is to check its partitions: > | MIN(last_analyzed) FROM pg_stat_all_tables psat JOIN pg_inherits i ON > psat.relid=i.inhrelid > > In 20200418050815.ge26...@telsasoft.com I wrote: > |This patch includes partitioned tables in pg_stat_*_tables, which is great; I > |complained awhile ago that they were missing [0]. It might be useful if that > |part was split out into a separate 0001 patch (?). > | [0] > https://www.postgresql.org/message-id/20180601221428.GU5164%40telsasoft.com
I suggest the attached (which partially reverts the revert), to allow showing correct data for analyze_count and last_analyzed. Arguably these should be reported as null in v14 for partitioned tables, since they're not "known to be zero", but rather "currently unpopulated". n_mod_since_analyze | 0 n_ins_since_vacuum | 0 Justin
>From 0d0e149727d89115803b4528e15f5b3c04bd816b Mon Sep 17 00:00:00 2001 From: Justin Pryzby <pryz...@telsasoft.com> Date: Mon, 16 Aug 2021 22:55:06 -0500 Subject: [PATCH] Report last_analyze and analyze_count of partitioned tables.. In v14, partitioned tables are included, but these fields are being reported as zero, which is misleading. --- src/backend/commands/analyze.c | 36 ++++++++++++++++++++++----------- src/backend/postmaster/pgstat.c | 27 +++++++++++++++++-------- 2 files changed, 43 insertions(+), 20 deletions(-) diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c index 8d7b38d170..0050df08f6 100644 --- a/src/backend/commands/analyze.c +++ b/src/backend/commands/analyze.c @@ -626,8 +626,8 @@ do_analyze_rel(Relation onerel, VacuumParams *params, PROGRESS_ANALYZE_PHASE_FINALIZE_ANALYZE); /* - * Update pages/tuples stats in pg_class, and report ANALYZE to the stats - * collector ... but not if we're doing inherited stats. + * Update pages/tuples stats in pg_class ... but not if we're doing + * inherited stats. * * We assume that VACUUM hasn't set pg_class.reltuples already, even * during a VACUUM ANALYZE. Although VACUUM often updates pg_class, @@ -668,20 +668,32 @@ do_analyze_rel(Relation onerel, VacuumParams *params, InvalidMultiXactId, in_outer_xact); } - + } + else if (onerel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) + { /* - * Now report ANALYZE to the stats collector. - * - * We deliberately don't report to the stats collector when doing - * inherited stats, 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. + * Partitioned tables don't have storage, so we don't set any fields + * in their pg_class entries except for reltuples, which is necessary + * for auto-analyze to work properly, and relhasindex. */ + vac_update_relstats(onerel, -1, totalrows, + 0, hasindex, InvalidTransactionId, + InvalidMultiXactId, + in_outer_xact); + } + + /* + * Now report ANALYZE to the stats collector. For regular tables, we do + * it only if not doing inherited stats. For partitioned tables, we only + * do it for inherited stats. (We're never called for not-inherited stats + * on partitioned tables anyway.) + * + * Reset the changes_since_analyze counter only if we analyzed all + * columns; otherwise, there is still work for auto-analyze to do. + */ + if (!inh || onerel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) pgstat_report_analyze(onerel, totalrows, totaldeadrows, (va_cols == NIL)); - } /* * If this isn't part of VACUUM ANALYZE, let index AMs do cleanup. diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c index a3c35bdf60..2a9673154b 100644 --- a/src/backend/postmaster/pgstat.c +++ b/src/backend/postmaster/pgstat.c @@ -1632,21 +1632,31 @@ pgstat_report_analyze(Relation rel, * be double-counted after commit. (This approach also ensures that the * collector ends up with the right numbers if we abort instead of * committing.) + * + * For partitioned tables, we don't report live and dead tuples, because + * such tables don't have any data. */ if (rel->pgstat_info != NULL) { PgStat_TableXactStatus *trans; - for (trans = rel->pgstat_info->trans; trans; trans = trans->upper) + if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) + /* If this rel is partitioned, skip modifying */ + livetuples = deadtuples = 0; + else { - livetuples -= trans->tuples_inserted - trans->tuples_deleted; - deadtuples -= trans->tuples_updated + trans->tuples_deleted; + for (trans = rel->pgstat_info->trans; trans; trans = trans->upper) + { + livetuples -= trans->tuples_inserted - trans->tuples_deleted; + deadtuples -= trans->tuples_updated + trans->tuples_deleted; + } + /* count stuff inserted by already-aborted subxacts, too */ + deadtuples -= rel->pgstat_info->t_counts.t_delta_dead_tuples; + /* Since ANALYZE's counts are estimates, we could have underflowed */ + livetuples = Max(livetuples, 0); + deadtuples = Max(deadtuples, 0); } - /* count stuff inserted by already-aborted subxacts, too */ - deadtuples -= rel->pgstat_info->t_counts.t_delta_dead_tuples; - /* Since ANALYZE's counts are estimates, we could have underflowed */ - livetuples = Max(livetuples, 0); - deadtuples = Max(deadtuples, 0); + } pgstat_setheader(&msg.m_hdr, PGSTAT_MTYPE_ANALYZE); @@ -1999,6 +2009,7 @@ pgstat_initstats(Relation rel) /* We only count stats for things that have storage */ if (!RELKIND_HAS_STORAGE(relkind)) +// relkind != RELKIND_PARTITIONED_TABLE) { rel->pgstat_info = NULL; return; -- 2.17.0