On 12/21/17, 11:07 PM, "Masahiko Sawada" <sawada.mshk@gmail.com> wrote:
> On the latest patch, it looks good to me except for a following comment.
>
> - * If the RangeVar is not defined, we do not have
> enough information
> - * to provide a meaningful log statement. Chances are that
> - * vacuum_rel's caller has intentionally not provided
> this information
> - * so that this logging is skipped, anyway.
> + * If relname is NULL, we do not have enough
> information to provide a
> + * meaningful log statement. Chances are that this
> information was
> + * intentionally not provided so that this logging is
> skipped, anyway.
>
> This comment looks odd to me because we ourselves didn't set relname
> just before. For example, we can move the sentence to above comment
> block as follows. Thoughts?
>
> /*
> * If relation is NULL, we do not have enough information to provide a
> * meaningful log statement. Chances are that this information was
> * intentionally not provided so that this logging is skipped, anyway.
> * However, iff VACOPT_NOWAIT is specified, we always try to emit
> * a log statement even if relation is NULL.
> */
>
> (snip)
>
> /*
> * Determine the log level.
> *
> * For autovacuum logs, we emit a LOG if
> * log_autovacuum_min_duration is not diabled. For manual VACUUM, we
> * emit a WARNING to match the log statements in the permission
> * checks.
> */
I agree, this makes more sense. I've made this change in v3 of 0003.
Nathan