pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
Fix regression in vacuumdb --analyze-in-stages for partitioned tables
5+ messages / 2 participants
[nested] [flat]

* Fix regression in vacuumdb --analyze-in-stages for partitioned tables
@ 2026-05-29 08:40 Chao Li <li.evan.chao@gmail.com>
  2026-05-29 16:08 ` Re: Fix regression in vacuumdb --analyze-in-stages for partitioned tables Fujii Masao <masao.fujii@gmail.com>
  0 siblings, 1 reply; 5+ messages in thread

From: Chao Li @ 2026-05-29 08:40 UTC (permalink / raw)
  To: pgsql-hackers; +Cc: Fujii Masao <masao.fujii@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Antonin Houska <ah@cybertec.at>; Laurenz Albe <laurenz.albe@cybertec.at>

Hi,

While testing "vacuumdb: Make vacuumdb --analyze-only process partitioned tables”, I found a regression from later commit c4067383cb2.

The original feature commit 6429e5b77 made "--analyze-in-stages" work for partitioned tables, as the doc change states:
```
--- a/doc/src/sgml/ref/vacuumdb.sgml
+++ b/doc/src/sgml/ref/vacuumdb.sgml
@@ -397,6 +397,15 @@ PostgreSQL documentation
         Multiple tables can be vacuumed by writing multiple
         <option>-t</option> switches.
        </para>
+       <para>
+        If no tables are specified with the <option>--table</option> option,
+        <application>vacuumdb</application> will clean all regular tables
+        and materialized views in the connected database.
+        If <option>--analyze-only</option> or
+        <option>--analyze-in-stages</option> is also specified,
+        it will analyze all regular tables, partitioned tables,
+        and materialized views (but not foreign tables).
+       </para>
```

The corresponding code was:
```
+               /*
+                * vacuumdb should generally follow the behavior of the underlying
+                * VACUUM and ANALYZE commands. If analyze_only is true, process
+                * regular tables, materialized views, and partitioned tables, just
+                * like ANALYZE (with no specific target tables) does. Otherwise,
+                * process only regular tables and materialized views, since VACUUM
+                * skips partitioned tables when no target tables are specified.
+                */
+               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");
```

However, the refactoring commit c4067383cb2 removed the `analyze_only` field from `vacuumingOptions` and switched to a new `mode` field. The new code is:
```
+               /*
+                * vacuumdb should generally follow the behavior of the underlying
+                * VACUUM and ANALYZE commands.  In MODE_ANALYZE mode, process regular
+                * tables, materialized views, and partitioned tables, just like
+                * ANALYZE (with no specific target tables) does. Otherwise, process
+                * only regular tables and materialized views, since VACUUM skips
+                * partitioned tables when no target tables are specified.
+                */
+               if (vacopts->mode == MODE_ANALYZE)
+                       appendPQExpBufferStr(&catalog_query,
+                                                                " AND c.relkind OPERATOR(pg_catalog.=) ANY (array["
+                                                                CppAsString2(RELKIND_RELATION) ", "
+                                                                CppAsString2(RELKIND_MATVIEW) ", "
+                                                                CppAsString2(RELKIND_PARTITIONED_TABLE) "])\n");
```

analyze_only used to be true when "--analyze-in-stages" was specified, but that meaning was lost in c4067383cb2:
```
                        case 3:
-                               analyze_in_stages = vacopts.analyze_only = true;
+                               vacopts.mode = MODE_ANALYZE_IN_STAGES;
                                break;
```

The fix is very straightforward, just add check for vacopts->mode == MODE_ANALYZE_IN_STAGES. I also added a test. If we had had this test earlier, the regression should have been caught.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

Attachments:

  [application/octet-stream] v1-0001-vacuumdb-Analyze-partitioned-tables-with-analyze-.patch (3.1K, ../../EDFF0AFB-050F-4FBF-8D4F-B44DC454D957@gmail.com/2-v1-0001-vacuumdb-Analyze-partitioned-tables-with-analyze-.patch)
  download | inline diff:
From bcef4667b618de169d17838f3edabdb90bc5f639 Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <lic@highgo.com>
Date: Fri, 29 May 2026 16:36:05 +0800
Subject: [PATCH v1] vacuumdb: Analyze partitioned tables with
 --analyze-in-stages

Commit 6429e5b77 made vacuumdb process partitioned tables when running in
analyze-only mode, including both --analyze-only and --analyze-in-stages.
This matched the documented behavior that, when no target tables are specified,
these options analyze regular tables, partitioned tables, and materialized
views.

Later, commit c4067383cb2 refactored vacuumingOptions by replacing the
analyze_only flag with a mode field.  During that refactoring, the object
selection logic was changed to check only MODE_ANALYZE, so
MODE_ANALYZE_IN_STAGES no longer included partitioned tables.

Fix this by treating MODE_ANALYZE_IN_STAGES the same as MODE_ANALYZE when
selecting objects to process, and add a regression test to cover the case.

Author: Chao Li <lic@highgo.com>
---
 src/bin/scripts/t/100_vacuumdb.pl |  4 ++++
 src/bin/scripts/vacuuming.c       | 14 ++++++++------
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/src/bin/scripts/t/100_vacuumdb.pl b/src/bin/scripts/t/100_vacuumdb.pl
index 84fcacd57fa..5fd55628507 100644
--- a/src/bin/scripts/t/100_vacuumdb.pl
+++ b/src/bin/scripts/t/100_vacuumdb.pl
@@ -363,6 +363,10 @@ $node->issues_sql_like(
 	[ 'vacuumdb', '--analyze-only', 'postgres' ],
 	qr/statement: ANALYZE public.parent_table/s,
 	'--analyze-only updates statistics for partitioned tables');
+$node->issues_sql_like(
+	[ 'vacuumdb', '--analyze-in-stages', 'postgres' ],
+	qr/statement: ANALYZE public.parent_table/s,
+	'--analyze-in-stages updates statistics for partitioned tables');
 $node->issues_sql_unlike(
 	[ 'vacuumdb', '--analyze-only', 'postgres' ],
 	qr/statement:\ VACUUM/sx,
diff --git a/src/bin/scripts/vacuuming.c b/src/bin/scripts/vacuuming.c
index faac9089a01..37608806056 100644
--- a/src/bin/scripts/vacuuming.c
+++ b/src/bin/scripts/vacuuming.c
@@ -650,13 +650,15 @@ retrieve_objects(PGconn *conn, vacuumingOptions *vacopts,
 	{
 		/*
 		 * vacuumdb should generally follow the behavior of the underlying
-		 * VACUUM and ANALYZE commands.  In MODE_ANALYZE mode, process regular
-		 * tables, materialized views, and partitioned tables, just like
-		 * ANALYZE (with no specific target tables) does. Otherwise, process
-		 * only regular tables and materialized views, since VACUUM skips
-		 * partitioned tables when no target tables are specified.
+		 * VACUUM and ANALYZE commands.  In MODE_ANALYZE or
+		 * MODE_ANALYZE_IN_STAGES modes, process regular tables, materialized
+		 * views, and partitioned tables, just like ANALYZE (with no specific
+		 * target tables) does. Otherwise, process only regular tables and
+		 * materialized views, since VACUUM skips partitioned tables when no
+		 * target tables are specified.
 		 */
-		if (vacopts->mode == MODE_ANALYZE)
+		if (vacopts->mode == MODE_ANALYZE ||
+			vacopts->mode == MODE_ANALYZE_IN_STAGES)
 			appendPQExpBufferStr(&catalog_query,
 								 " AND c.relkind OPERATOR(pg_catalog.=) ANY (array["
 								 CppAsString2(RELKIND_RELATION) ", "
-- 
2.50.1 (Apple Git-155)

=

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

* Re: Fix regression in vacuumdb --analyze-in-stages for partitioned tables
  2026-05-29 08:40 Fix regression in vacuumdb --analyze-in-stages for partitioned tables Chao Li <li.evan.chao@gmail.com>
@ 2026-05-29 16:08 ` Fujii Masao <masao.fujii@gmail.com>
  2026-06-02 05:57   ` Re: Fix regression in vacuumdb --analyze-in-stages for partitioned tables Chao Li <li.evan.chao@gmail.com>
  0 siblings, 1 reply; 5+ messages in thread

From: Fujii Masao @ 2026-05-29 16:08 UTC (permalink / raw)
  To: Chao Li <li.evan.chao@gmail.com>; +Cc: pgsql-hackers; Álvaro Herrera <alvherre@kurilemu.de>; Antonin Houska <ah@cybertec.at>; Laurenz Albe <laurenz.albe@cybertec.at>

On Fri, May 29, 2026 at 5:41 PM Chao Li <li.evan.chao@gmail.com> wrote:
> The fix is very straightforward, just add check for vacopts->mode == MODE_ANALYZE_IN_STAGES. I also added a test. If we had had this test earlier, the regression should have been caught.

Thanks for reporting the issue and providing the patch! The patch
looks good to me.

Regards,

-- 
Fujii Masao





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

* Re: Fix regression in vacuumdb --analyze-in-stages for partitioned tables
  2026-05-29 08:40 Fix regression in vacuumdb --analyze-in-stages for partitioned tables Chao Li <li.evan.chao@gmail.com>
  2026-05-29 16:08 ` Re: Fix regression in vacuumdb --analyze-in-stages for partitioned tables Fujii Masao <masao.fujii@gmail.com>
@ 2026-06-02 05:57   ` Chao Li <li.evan.chao@gmail.com>
  2026-06-03 07:55     ` Re: Fix regression in vacuumdb --analyze-in-stages for partitioned tables Fujii Masao <masao.fujii@gmail.com>
  0 siblings, 1 reply; 5+ messages in thread

From: Chao Li @ 2026-06-02 05:57 UTC (permalink / raw)
  To: Fujii Masao <masao.fujii@gmail.com>; +Cc: pgsql-hackers; Álvaro Herrera <alvherre@kurilemu.de>; Antonin Houska <ah@cybertec.at>; Laurenz Albe <laurenz.albe@cybertec.at>



> On May 30, 2026, at 00:08, Fujii Masao <masao.fujii@gmail.com> wrote:
> 
> On Fri, May 29, 2026 at 5:41 PM Chao Li <li.evan.chao@gmail.com> wrote:
>> The fix is very straightforward, just add check for vacopts->mode == MODE_ANALYZE_IN_STAGES. I also added a test. If we had had this test earlier, the regression should have been caught.
> 
> Thanks for reporting the issue and providing the patch! The patch
> looks good to me.
> 
> Regards,
> 
> -- 
> Fujii Masao

For tracking purpose, I just added this to v19 open items.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/









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

* Re: Fix regression in vacuumdb --analyze-in-stages for partitioned tables
  2026-05-29 08:40 Fix regression in vacuumdb --analyze-in-stages for partitioned tables Chao Li <li.evan.chao@gmail.com>
  2026-05-29 16:08 ` Re: Fix regression in vacuumdb --analyze-in-stages for partitioned tables Fujii Masao <masao.fujii@gmail.com>
  2026-06-02 05:57   ` Re: Fix regression in vacuumdb --analyze-in-stages for partitioned tables Chao Li <li.evan.chao@gmail.com>
@ 2026-06-03 07:55     ` Fujii Masao <masao.fujii@gmail.com>
  2026-06-03 08:18       ` Re: Fix regression in vacuumdb --analyze-in-stages for partitioned tables Chao Li <li.evan.chao@gmail.com>
  0 siblings, 1 reply; 5+ messages in thread

From: Fujii Masao @ 2026-06-03 07:55 UTC (permalink / raw)
  To: Chao Li <li.evan.chao@gmail.com>; +Cc: pgsql-hackers; Álvaro Herrera <alvherre@kurilemu.de>; Antonin Houska <ah@cybertec.at>; Laurenz Albe <laurenz.albe@cybertec.at>

On Tue, Jun 2, 2026 at 2:57 PM Chao Li <li.evan.chao@gmail.com> wrote:
> > Thanks for reporting the issue and providing the patch! The patch
> > looks good to me.

I've pushed the patch. Thanks!

Regards,

-- 
Fujii Masao





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

* Re: Fix regression in vacuumdb --analyze-in-stages for partitioned tables
  2026-05-29 08:40 Fix regression in vacuumdb --analyze-in-stages for partitioned tables Chao Li <li.evan.chao@gmail.com>
  2026-05-29 16:08 ` Re: Fix regression in vacuumdb --analyze-in-stages for partitioned tables Fujii Masao <masao.fujii@gmail.com>
  2026-06-02 05:57   ` Re: Fix regression in vacuumdb --analyze-in-stages for partitioned tables Chao Li <li.evan.chao@gmail.com>
  2026-06-03 07:55     ` Re: Fix regression in vacuumdb --analyze-in-stages for partitioned tables Fujii Masao <masao.fujii@gmail.com>
@ 2026-06-03 08:18       ` Chao Li <li.evan.chao@gmail.com>
  0 siblings, 0 replies; 5+ messages in thread

From: Chao Li @ 2026-06-03 08:18 UTC (permalink / raw)
  To: Fujii Masao <masao.fujii@gmail.com>; +Cc: pgsql-hackers; Álvaro Herrera <alvherre@kurilemu.de>; Antonin Houska <ah@cybertec.at>; Laurenz Albe <laurenz.albe@cybertec.at>



> On Jun 3, 2026, at 15:55, Fujii Masao <masao.fujii@gmail.com> wrote:
> 
> On Tue, Jun 2, 2026 at 2:57 PM Chao Li <li.evan.chao@gmail.com> wrote:
>>> Thanks for reporting the issue and providing the patch! The patch
>>> looks good to me.
> 
> I've pushed the patch. Thanks!
> 
> Regards,
> 
> -- 
> Fujii Masao

Thanks for pushing.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/









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


end of thread, other threads:[~2026-06-03 08:18 UTC | newest]

Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-05-29 08:40 Fix regression in vacuumdb --analyze-in-stages for partitioned tables Chao Li <li.evan.chao@gmail.com>
2026-05-29 16:08 ` Fujii Masao <masao.fujii@gmail.com>
2026-06-02 05:57   ` Chao Li <li.evan.chao@gmail.com>
2026-06-03 07:55     ` Fujii Masao <masao.fujii@gmail.com>
2026-06-03 08:18       ` Chao Li <li.evan.chao@gmail.com>

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