Re: New vacuum option to do only freezing

On Fri, Mar 29, 2019 at 2:16 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> Attached updated patches. These patches are applied on top of 0001
> patch on parallel vacuum thread[1].

+    bool index_cleanup = true;  /* by default */

I think we should instead initialize index_cleanup to the reloption
value, if there is one, or true if none, and then let it be overridden
by the loop that follows, where whatever the user specifies in the SQL
command is processed.  That way, any explicitly-specified option
within the command itself wins, and the reloption sets the default.
As you have it, index cleanup is disabled when the reloption is set to
false even if the user wrote VACUUM (INDEX_CLEANUP TRUE).

+            appendStringInfo(&buf,
+                             _("%.0f tuples and %.0f item identifiers are left
as dead.\n"),
+                             vacrelstats->nleft_dead_tuples,
+                             vacrelstats->nleft_dead_itemids);

I tend to think we should omit this line entirely if both values are
0, as will very often be the case.

+    if ((params->options & VACOPT_FULL) != 0 &&
+        (params->options & VACOPT_INDEX_CLEANUP) == 0)
+        ereport(ERROR,
+                (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                 errmsg("VACUUM option INDEX_CLEANUP cannot be set to
false with FULL")));

I think it would be better to just ignore the INDEX_CLEANUP option
when FULL is specified.

I wasn't all that happy with the documentation changes you proposed.
Please find attached a proposed set of doc changes.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company