On Tue, Apr 14, 2020 at 7:52 AM Michael Paquier <mich...@paquier.xyz> wrote:
>
> Makes sense. I have two comments.
>
> ereport(ERROR,
> (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> - errmsg("cannot specify both FULL and PARALLEL options")));
> + errmsg("VACUUM FULL cannot be performed in parallel")));
> Better to avoid a full sentence here [1]? This should be a "cannot do
> foo" errror.
>
I could see similar error messages in other places like below:
CONCURRENTLY cannot be used when the materialized view is not populated
CONCURRENTLY and WITH NO DATA options cannot be used together
COPY delimiter cannot be newline or carriage return
Also, I am not sure if it violates the style we have used in code. It
seems the error message is short, succinct and conveys the required
information.
> -VACUUM (PARALLEL 1) tmp; -- disables parallel vacuum option
> +VACUUM (PARALLEL 1) tmp; -- parallel vacuum disabled for temp tables
> WARNING: disabling parallel option of vacuum on "tmp" --- cannot vacuum
> temporary tables in parallel
> +VACUUM (PARALLEL 0, FULL TRUE) tmp; -- can specify parallel disabled (even
> though that's implied by FULL)
>
> To fully close the gap in the tests, I would also add a test for
> (PARALLEL 1, FULL false) where FULL directly specified, even if that
> sounds like a nit. That's fine to test even on a temporary table.
>
Okay, I will do this once we agree on the error message stuff.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
