Re: New vacuum option to do only freezing

On Fri, Mar 29, 2019 at 10:46 PM Robert Haas <robertmhaas@gmail.com> wrote:
>
> 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).
>

Yeah, but since multiple relations might be specified in VACUUM
command we need to process index_cleanup option after opened each
relations. Maybe we need to process all options except for
INDEX_CLEANUP in ExecVacuum() and pass VacuumStmt down to vacuum_rel()
and process it  in manner of you suggested after opened the relation.
Is that right?

> +            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.

Fixed.

>
> +    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.

Okay, but why do we ignore that in this case while we complain in the
case of FULL and DISABLE_PAGE_SKIPPING?

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

Thank you! I've incorporated these changes.
Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center