On Mon, Mar 06, 2023 at 03:48:28PM -0500, Melanie Plageman wrote: > On Mon, Mar 6, 2023 at 3:27 PM Nathan Bossart <nathandboss...@gmail.com> > wrote: >> On Mon, Mar 06, 2023 at 02:40:09PM -0500, Melanie Plageman wrote: >> > I noticed in vacuum_rel() in vacuum.c where table_relation_vacuum() is >> > called, 4211fbd84 changes the else into an else if [1]. I understand >> > after reading the commit and re-reading the code why that is now, but I >> > was initially confused. I was thinking it might be nice to have a >> > comment mentioning why there is no else case here (i.e. that the main >> > table relation will be vacuumed on the else if branch). >> >> This was a hack to avoid another level of indentation for that whole block >> of code, but based on your comment, it might be better to just surround >> this entire section with an "if (params->options & VACOPT_PROCESS_MAIN)" >> check. WDYT? > > I think that would be clearer.
Here's a patch. Thanks for reviewing. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index 580f966499..fb1ef28fa9 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -2060,23 +2060,25 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params, bool skip_privs) /* * Do the actual work --- either FULL or "lazy" vacuum */ - if ((params->options & VACOPT_FULL) && - (params->options & VACOPT_PROCESS_MAIN)) + if (params->options & VACOPT_PROCESS_MAIN) { - ClusterParams cluster_params = {0}; + if (params->options & VACOPT_FULL) + { + ClusterParams cluster_params = {0}; - /* close relation before vacuuming, but hold lock until commit */ - relation_close(rel, NoLock); - rel = NULL; + /* close relation before vacuuming, but hold lock until commit */ + relation_close(rel, NoLock); + rel = NULL; - if ((params->options & VACOPT_VERBOSE) != 0) - cluster_params.options |= CLUOPT_VERBOSE; + if ((params->options & VACOPT_VERBOSE) != 0) + cluster_params.options |= CLUOPT_VERBOSE; - /* VACUUM FULL is now a variant of CLUSTER; see cluster.c */ - cluster_rel(relid, InvalidOid, &cluster_params); + /* VACUUM FULL is now a variant of CLUSTER; see cluster.c */ + cluster_rel(relid, InvalidOid, &cluster_params); + } + else + table_relation_vacuum(rel, params, vac_strategy); } - else if (params->options & VACOPT_PROCESS_MAIN) - table_relation_vacuum(rel, params, vac_strategy); /* Roll back any GUC changes executed by index functions */ AtEOXact_GUC(false, save_nestlevel);