Re: add PROCESS_MAIN to VACUUM

Thanks for taking a look.

On Wed, Mar 01, 2023 at 03:31:48PM +0900, Michael Paquier wrote:
> PROCESS_TOAST has that:
> /* sanity check for PROCESS_TOAST */
> if ((params->options & VACOPT_FULL) != 0 &&
> (params->options & VACOPT_PROCESS_TOAST) == 0)
> ereport(ERROR,
> (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> errmsg("PROCESS_TOAST required with VACUUM FULL")));
> [...]
> - if (params->options & VACOPT_FULL)
> + if (params->options & VACOPT_FULL &&
> + params->options & VACOPT_PROCESS_MAIN)
> {
>
> Shouldn't we apply the same rule for PROCESS_MAIN? One of the
> regression tests added means that FULL takes priority over
> PROCESS_MAIN=FALSE, which is a bit confusing IMO.

I don't think so. We disallow FULL without PROCESS_TOAST because there
presently isn't a way to VACUUM FULL the main relation without rebuilding
its TOAST table. However, FULL without PROCESS_MAIN can be used to run
VACUUM FULL on only the TOAST table.

> @@ -190,6 +190,7 @@ typedef struct VacAttrStats
> #define VACOPT_DISABLE_PAGE_SKIPPING 0x80 /* don't skip any pages */
> #define VACOPT_SKIP_DATABASE_STATS 0x100 /* skip vac_update_datfrozenxid() */
> #define VACOPT_ONLY_DATABASE_STATS 0x200 /* only vac_update_datfrozenxid() */
> +#define VACOPT_PROCESS_MAIN 0x400 /* process main relation */
>
> Perhaps the options had better be reorganized so as PROCESS_MAIN is
> just before PROCESS_TOAST?

Sure.

> +-- PROCESS_MAIN option
> +VACUUM (PROCESS_MAIN FALSE) vactst;
> +VACUUM (PROCESS_MAIN FALSE, PROCESS_TOAST FALSE) vactst;
> +VACUUM (PROCESS_MAIN FALSE, FULL) vactst;
>
> Thinking a bit here. This set of tests does not make sure that the
> main relation and/or the toast relation have been actually processed.
> pg_stat_user_tables does not track what's happening on the toast
> relations. So... What about adding some tests in 100_vacuumdb.pl
> that rely on vacuumdb --verbose and check the logs produced? We
> should make sure that the toast or the main relation are processed,
> by tracking, for example, logs like vacuuming "schema.table". When
> FULL is involved, we may want to track the changes on relfilenodes
> depending on what's wanted.

That seems like a good idea.

--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com