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/




Attachment: v1-0001-vacuumdb-Analyze-partitioned-tables-with-analyze-.patch
Description: Binary data

Reply via email to