Date: Wed, 8 Apr 2020 11:41:54 -0500
From: Justin Pryzby <pryzby@telsasoft.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Mahendra Singh Thalor <mahi6run@gmail.com>,
	tushar <tushar.ahuja@enterprisedb.com>,
	pgsql-hackers@postgresql.org
Subject: Re: Vacuum o/p with (full 1, parallel 0) option throwing an error
Message-ID: <20200408164154.GD2228@telsasoft.com>
References: <58c8d171-e665-6fa3-a9d3-d9423b694dae@enterprisedb.com>
 <CA+TgmoY6JBix-20yS=EicJVHNAH44QnYGQhyR_c15dZi02KF-w@mail.gmail.com>
 <CAKYtNAqzJ8rTh9Mb+2XdsH0dyZb2MS_XH-w8vz3i347etnaFSQ@mail.gmail.com>
 <CA+TgmoYkFYRnmTLgPLPm664UdQFBcJTcDZXzBP6dG0zTniYEWQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="Jl+DbTnyraiZ/loT"
Content-Disposition: inline
In-Reply-To: 
 <CA+TgmoYkFYRnmTLgPLPm664UdQFBcJTcDZXzBP6dG0zTniYEWQ@mail.gmail.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk


--Jl+DbTnyraiZ/loT
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Wed, Apr 08, 2020 at 11:57:08AM -0400, Robert Haas wrote:
> On Wed, Apr 8, 2020 at 10:25 AM Mahendra Singh Thalor
> <mahi6run@gmail.com> wrote:
> > I think, Tushar point is that either we should allow both
> > vacuum(parallel 0, full 1) and vacuum(parallel 1, full 0) or in the
> > both cases, we should through error.
> 
> Oh, yeah, good point. Somebody must not've been careful enough with
> the options-checking code.

Actually I think someone was too careful.

From 9256cdb0a77fb33194727e265a346407921055ef Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Wed, 8 Apr 2020 11:38:36 -0500
Subject: [PATCH v1] parallel vacuum: options check to use same test as in
 vacuumlazy.c

---
 src/backend/commands/vacuum.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 351d5215a9..660c854d49 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -104,7 +104,6 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
 	bool		freeze = false;
 	bool		full = false;
 	bool		disable_page_skipping = false;
-	bool		parallel_option = false;
 	ListCell   *lc;
 
 	/* Set default value */
@@ -145,7 +144,6 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
 			params.truncate = get_vacopt_ternary_value(opt);
 		else if (strcmp(opt->defname, "parallel") == 0)
 		{
-			parallel_option = true;
 			if (opt->arg == NULL)
 			{
 				ereport(ERROR,
@@ -199,7 +197,7 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
 		   !(params.options & (VACOPT_FULL | VACOPT_FREEZE)));
 	Assert(!(params.options & VACOPT_SKIPTOAST));
 
-	if ((params.options & VACOPT_FULL) && parallel_option)
+	if ((params.options & VACOPT_FULL) && params.nworkers > 0)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("cannot specify both FULL and PARALLEL options")));
-- 
2.17.0


--Jl+DbTnyraiZ/loT
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v1-0001-parallel-vacuum-options-check-to-use-same-test-as.patch"

From 9256cdb0a77fb33194727e265a346407921055ef Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Wed, 8 Apr 2020 11:38:36 -0500
Subject: [PATCH v1] parallel vacuum: options check to use same test as in
 vacuumlazy.c

---
 src/backend/commands/vacuum.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 351d5215a9..660c854d49 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -104,7 +104,6 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
 	bool		freeze = false;
 	bool		full = false;
 	bool		disable_page_skipping = false;
-	bool		parallel_option = false;
 	ListCell   *lc;
 
 	/* Set default value */
@@ -145,7 +144,6 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
 			params.truncate = get_vacopt_ternary_value(opt);
 		else if (strcmp(opt->defname, "parallel") == 0)
 		{
-			parallel_option = true;
 			if (opt->arg == NULL)
 			{
 				ereport(ERROR,
@@ -199,7 +197,7 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
 		   !(params.options & (VACOPT_FULL | VACOPT_FREEZE)));
 	Assert(!(params.options & VACOPT_SKIPTOAST));
 
-	if ((params.options & VACOPT_FULL) && parallel_option)
+	if ((params.options & VACOPT_FULL) && params.nworkers > 0)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("cannot specify both FULL and PARALLEL options")));
-- 
2.17.0


--Jl+DbTnyraiZ/loT--