Date: Mon, 6 Mar 2023 14:43:10 -0800
From: Nathan Bossart <nathandbossart@gmail.com>
To: pgsql-hackers@postgresql.org, michael@paquier.xyz
Subject: Re: add PROCESS_MAIN to VACUUM
Message-ID: <20230306224310.GA3165453@nathanxps13>
References: <20230301172637.GA1456203@nathanxps13>
 <ZAA5vt7jUTRpPDmL@paquier.xyz>
 <20230302065359.GA2140777@nathanxps13>
 <ZAWbkiZSdFZOTS7F@paquier.xyz>
 <20230306173723.GA2934573@nathanxps13>
 <20230306194009.5cn6sp3wjotd36nu@liskov>
 <20230306202734.GA3033389@nathanxps13>
 <CAAKRu_ZJMFrO_tspzoX43h9NyEFS1nS438MoS3Yv7JxK3=jFPw@mail.gmail.com>
 <20230306211337.GA3076909@nathanxps13>
 <20230306220958.rk4b4msmybi6hm56@liskov>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="17pEHd4RhPHOinZp"
Content-Disposition: inline
In-Reply-To: <20230306220958.rk4b4msmybi6hm56@liskov>
Archived-At: 
 <https://www.postgresql.org/message-id/20230306224310.GA3165453%40nathanxps13>
Precedence: bulk


--17pEHd4RhPHOinZp
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Mon, Mar 06, 2023 at 05:09:58PM -0500, Melanie Plageman wrote:
> I would move this comment inside of the outer if statement since it is
> distinguishing between the two branches of the inner if statement.

Oops, done.

> Also, I would still consider putting a comment above that reminds us that
> VACOPT_PROCESS_MAIN is the default and will vacuum the main relation.

I tried adding something along these lines.

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

--17pEHd4RhPHOinZp
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="process_main_readability_fix_v2.patch"

diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 580f966499..90a42e089c 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -2058,25 +2058,32 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params, bool skip_privs)
 	save_nestlevel = NewGUCNestLevel();
 
 	/*
-	 * Do the actual work --- either FULL or "lazy" vacuum
+	 * If PROCESS_MAIN is set (the default), it's time to vacuum the main
+	 * relation.  Otherwise, we can skip this part.  If required, we'll process
+	 * the TOAST table later.
 	 */
-	if ((params->options & VACOPT_FULL) &&
-		(params->options & VACOPT_PROCESS_MAIN))
+	if (params->options & VACOPT_PROCESS_MAIN)
 	{
-		ClusterParams cluster_params = {0};
+		/*
+		 * Do the actual work --- either FULL or "lazy" vacuum
+		 */
+		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);

--17pEHd4RhPHOinZp--