public inbox for [email protected]  
help / color / mirror / Atom feed
Re: analyze-in-stages post upgrade questions
2+ messages / 2 participants
[nested] [flat]

* Re: analyze-in-stages post upgrade questions
@ 2025-06-28 05:25  Laurenz Albe <[email protected]>
  0 siblings, 1 reply; 2+ messages in thread

From: Laurenz Albe @ 2025-06-28 05:25 UTC (permalink / raw)
  To: Zechman, Derek S <[email protected]>; Adrian Klaver <[email protected]>; [email protected] <[email protected]>

On Sat, 2025-06-28 at 01:23 +0000, Zechman, Derek S wrote:
> > Well, that wouldn't explain why it doesn't work on partitioned tables.
> > I am under the impression that it should.
> >
> > Derek, can cou share the pg_stats entries for the partitioned table?
> 
> There are no entries in pg_stats for the parent table until after I manually run an analyze on it – Example below

You are right.  I looked at the code, and "vacuumdb" does not process
partitiond tables, even if --analyze-only is specified.  I find that
surprising, aince the SQL command ANALYZE (without a table name) will
also collect statistics for partitioned tables.

I think that it would be a good idea to change that behavior.
In particular, it makes a lot of sense to collect statistics for
partitioned tables after a "pg_upgrade".

Attached is a patch to make "vacuumdb --analyze-only" consider
partitioned tables as well.

Yours,
Laurenz Albe


Attachments:

  [text/x-patch] v1-0001-Make-vacuumdb-Z-process-partitioned-tables.patch (3.0K, 2-v1-0001-Make-vacuumdb-Z-process-partitioned-tables.patch)
  download | inline diff:
From 07cbd491011ff5da1243b117c111d51531293782 Mon Sep 17 00:00:00 2001
From: Laurenz Albe <[email protected]>
Date: Sat, 28 Jun 2025 07:20:02 +0200
Subject: [PATCH v1] Make vacuumdb -Z process partitioned tables

Autoanalyze won't process partitioned tables, but a manual ANALYZE
of the whole database does.  So it was surprising that partitioned
tables were not processed by "vacuumdb --analyze-only".

In addition, "vacuumdb --analyze-only" is what you run to collect
missing statistics after a "pg_upgrade", and it makes a lot of sense
to collect statistics for partitioned tables at that point.

However, running VACUUM on partitioned tables adds no benefit over
vacuuming the partitions, and VACUUM is more expensive than ANALYZE,
so we won't treat partitioned tables unless --analyze-only is given.
Otherwise, we'd end up vacuuming the partitions twice, which would
be a waste of resources.
---
 doc/src/sgml/ref/vacuumdb.sgml |  4 ++++
 src/bin/scripts/vacuumdb.c     | 23 +++++++++++++++++++----
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/doc/src/sgml/ref/vacuumdb.sgml b/doc/src/sgml/ref/vacuumdb.sgml
index b0680a61814..6a4c8d51b7f 100644
--- a/doc/src/sgml/ref/vacuumdb.sgml
+++ b/doc/src/sgml/ref/vacuumdb.sgml
@@ -440,6 +440,10 @@ PostgreSQL documentation
       <listitem>
        <para>
         Only calculate statistics for use by the optimizer (no vacuum).
+        If that option is specified, <command>vacuumdb</command> will also
+        process partitioned tables.  Without that option, only the partitions
+        will be considered, unless a partitioned table is explicitly specified
+        with the <option>--table</option> option.
        </para>
       </listitem>
      </varlistentry>
diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c
index 79b1096eb08..d7ca8300f43 100644
--- a/src/bin/scripts/vacuumdb.c
+++ b/src/bin/scripts/vacuumdb.c
@@ -911,10 +911,25 @@ retrieve_objects(PGconn *conn, vacuumingOptions *vacopts,
 	 */
 	if ((objfilter & OBJFILTER_TABLE) == 0)
 	{
-		appendPQExpBufferStr(&catalog_query,
-							 " AND c.relkind OPERATOR(pg_catalog.=) ANY (array["
-							 CppAsString2(RELKIND_RELATION) ", "
-							 CppAsString2(RELKIND_MATVIEW) "])\n");
+		/*
+		 * VACUUMing partitioned tables would be unreasonably expensive, since
+		 * that entails processing the partitions twice (once as part of the
+		 * partitioned table, once as tables in their own right) for no
+		 * benefit. But if we only ANALYZE, collecting statistics for
+		 * partitioned tables is worth the effort.
+		 */
+		if (vacopts->analyze_only)
+			appendPQExpBufferStr(&catalog_query,
+								 " AND c.relkind OPERATOR(pg_catalog.=) ANY (array["
+								 CppAsString2(RELKIND_RELATION) ", "
+								 CppAsString2(RELKIND_MATVIEW) ", "
+								 CppAsString2(RELKIND_PARTITIONED_TABLE) "])\n");
+		else
+			appendPQExpBufferStr(&catalog_query,
+								 " AND c.relkind OPERATOR(pg_catalog.=) ANY (array["
+								 CppAsString2(RELKIND_RELATION) ", "
+								 CppAsString2(RELKIND_MATVIEW) "])\n");
+
 	}
 
 	/*
-- 
2.50.0



^ permalink  raw  reply  [nested|flat] 2+ messages in thread

* Re: analyze-in-stages post upgrade questions
@ 2025-07-07 12:43  Mircea Cadariu <[email protected]>
  parent: Laurenz Albe <[email protected]>
  0 siblings, 0 replies; 2+ messages in thread

From: Mircea Cadariu @ 2025-07-07 12:43 UTC (permalink / raw)
  To: [email protected]; +Cc: Laurenz Albe <[email protected]>

The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:       tested, passed
Spec compliant:           tested, passed
Documentation:            tested, passed

Hi Laurenz,

Nice focused patch. 

Applied successfully on commit 62a17a92 from master. 

Documentation is updated and there is an explanatory comment for the code, as well as a descriptive commit message. 

To check the patch, I added the following test at the end of src/bin/scripts/t/100_vacuumdb.pl in both master (as experiment) and your patch. In master it does not pass, but with your patch applied it does. 

You can consider adding it to your patch, or I could also do that. 

$node->safe_psql('postgres',
    "CREATE TABLE parent_table (a INT) PARTITION BY LIST (a);\n"
      . "CREATE TABLE child_table PARTITION OF parent_table FOR VALUES IN (1);\n"
      . "INSERT INTO parent_table VALUES (1);\n");
$node->issues_sql_like(
    [
        'vacuumdb', '--analyze-only', 'postgres'
    ],
    qr/statement:\s+ANALYZE\s+public\.parent_table/s,
    '--analyze_only updates statistics for partitioned tables');

Kind regards,
Mircea Cadariu

The new status of this patch is: Waiting on Author


^ permalink  raw  reply  [nested|flat] 2+ messages in thread


end of thread, other threads:[~2025-07-07 12:43 UTC | newest]

Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-06-28 05:25 Re: analyze-in-stages post upgrade questions Laurenz Albe <[email protected]>
2025-07-07 12:43 ` Mircea Cadariu <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox