Fix regression in vacuumdb --analyze-in-stages for partitioned tables - Mailing list pgsql-hackers

From Chao Li
Subject Fix regression in vacuumdb --analyze-in-stages for partitioned tables
Date
Msg-id EDFF0AFB-050F-4FBF-8D4F-B44DC454D957@gmail.com
Whole thread
Responses Re: Fix regression in vacuumdb --analyze-in-stages for partitioned tables
List pgsql-hackers
Hi,

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

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
hadhad this test earlier, the regression should have been caught. 

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





Attachment

pgsql-hackers by date:

Previous
From: Peter Smith
Date:
Subject: Re: Support EXCEPT for TABLES IN SCHEMA publications
Next
From: "Joel Jacobson"
Date:
Subject: Re: Key joins