Re: [HACKERS] [Proposal] Allow users to specify multiple tables inVACUUM commands - Mailing list pgsql-hackers

On Fri, May 12, 2017 at 9:47 AM, Bossart, Nathan <bossartn@amazon.com> wrote:
> Attached is a more complete first attempt at adding this functionality.  I added two node types: one for parsing the
“relationand columns” list in the grammar, and one for holding the relation information we need for each call to
vacuum_rel(…)/analyze_rel(…). I also added assertions and comments for some undocumented assumptions that we currently
relyupon. 
>
> Adjustments to the documentation for VACUUM/ANALYZE and new checks in the VACUUM regression test are included in this
patchas well. 
>
> Looking forward to any feedback that you have.

Browsing the code....
<synopsis>
-ANALYZE [ VERBOSE ] [ <replaceable
class="PARAMETER">table_name</replaceable> [ ( <replaceable
class="PARAMETER">column_name</replaceable> [, ...] ) ] ]
+ANALYZE [ VERBOSE ] [ <replaceable
class="PARAMETER">table_name</replaceable> [ [ ( <replaceable
class="PARAMETER">column_name</replaceable> [, ...] ) ] [, ...] ]</synopsis>
It seems to me that you don't need those extra square brackets around
the column list. Same remark for vacuum.sgml.
    <listitem>     <para>
-      The name (possibly schema-qualified) of a specific table to
+      The name (possibly schema-qualified) of the specific tables to      analyze.  If omitted, all regular tables,
partitionedtables, and      materialized views in the current database are analyzed (but not 
-      foreign tables).  If the specified table is a partitioned table, both the
+      foreign tables).  If a specified table is a partitioned table, both the      inheritance statistics of the
partitionedtable as a whole and      statistics of the individual partitions are updated.     </para> 
Don't think that's needed. table_name is still referencing a single
table name. And similar remark for vacuum.sgml.

In short for all that, it is enough to have "[, ... ]" to document
that a list is accepted.
   /* Now go through the common routine */
-   vacuum(vacstmt->options, vacstmt->relation, InvalidOid, ¶ms,
-          vacstmt->va_cols, NULL, isTopLevel);
+   vacuum(vacstmt->options, vacstmt->rels, InvalidOid, ¶ms,
+          NULL, isTopLevel);}
It seems to me that it would have been less invasive to loop through
vacuum() for each relation. Do you foresee advantages in allowing
vacuum() to handle multiple? I am not sure if is is worth complicating
the current logic more considering that you have as well so extra
logic to carry on option values.

+ * used for error messages.  In the case that relid is valid, rels
+ * must have exactly one element corresponding to the specified relid.
s/rels/relations/ as variable name?
--
Michael