Date: Thu, 9 Apr 2020 03:33:19 -0500
From: Justin Pryzby <pryzby@telsasoft.com>
To: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>,
	Mahendra Singh Thalor <mahi6run@gmail.com>,
	Robert Haas <robertmhaas@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: <20200409083319.GN2228@telsasoft.com>
References: 
 <CA+TgmoY6JBix-20yS=EicJVHNAH44QnYGQhyR_c15dZi02KF-w@mail.gmail.com>
 <CAKYtNAqzJ8rTh9Mb+2XdsH0dyZb2MS_XH-w8vz3i347etnaFSQ@mail.gmail.com>
 <CA+TgmoYkFYRnmTLgPLPm664UdQFBcJTcDZXzBP6dG0zTniYEWQ@mail.gmail.com>
 <20200408164154.GD2228@telsasoft.com>
 <CAKYtNAogYT+ZNN9Q3eCpdWHVsYR5R-o5p0NtOZHLOkYP-4dxsA@mail.gmail.com>
 <20200408183854.GG2228@telsasoft.com>
 <CAA4eK1J-CQ4_jt85um8WSeCRjkC4aEtB+Ft43_tFHLVS8n_CQA@mail.gmail.com>
 <CA+fd4k5TNziHyAYaeFyL8jHN9aiGnNPv7SJ5YGqj92uZyA_RfQ@mail.gmail.com>
 <CAA4eK1LYqw_MN9=MHcxF6ayvmN2NdaxaqJ5e+f2GobBpfC1AjA@mail.gmail.com>
 <CA+fd4k7cj3rO3prCydHQd_pXShQ2rYbUyEJWeE8o+sXtB_1xmg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="YC8Ek3FeOE8ywfXk"
Content-Disposition: inline
In-Reply-To: 
 <CA+fd4k7cj3rO3prCydHQd_pXShQ2rYbUyEJWeE8o+sXtB_1xmg@mail.gmail.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk


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

On Thu, Apr 09, 2020 at 05:07:48PM +0900, Masahiko Sawada wrote:
> Yes but the difference is that we cannot disable PARSER or COPY by
> specifying options whereas we can do something like "VACUUM (FULL
> false) tbl" to disable FULL option. I might be misunderstanding the
> meaning of "specify" though.

You have it right.

We should fix the behavior, but change the error message for consistency with
that change, like so.

-- 
Justin

--YC8Ek3FeOE8ywfXk
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v2-0001-Allow-specifying-parallel-0-with-vacuum-full.patch"

From 206d4f2e05360e9d02bcf3b941f9f8ea99f5f8a9 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Thu, 9 Apr 2020 03:29:13 -0500
Subject: [PATCH v2] Allow specifying "(parallel 0)" with "vacuum(full)"..

..even though full implies parallel 0

Discussion:
https://www.postgresql.org/message-id/58c8d171-e665-6fa3-a9d3-d9423b694dae%40enterprisedb.com
---
 src/backend/commands/vacuum.c        | 2 +-
 src/test/regress/expected/vacuum.out | 5 +++--
 src/test/regress/sql/vacuum.sql      | 3 ++-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 660c854d49..645162d4bd 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -200,7 +200,7 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
 	if ((params.options & VACOPT_FULL) && params.nworkers > 0)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("cannot specify both FULL and PARALLEL options")));
+				 errmsg("cannot specify both FULL and parallel workers")));
 
 	/*
 	 * Make sure VACOPT_ANALYZE is specified if any column lists are present.
diff --git a/src/test/regress/expected/vacuum.out b/src/test/regress/expected/vacuum.out
index 0cfe28e63f..bc2c3db8d8 100644
--- a/src/test/regress/expected/vacuum.out
+++ b/src/test/regress/expected/vacuum.out
@@ -115,14 +115,15 @@ LINE 1: VACUUM (PARALLEL -1) pvactst;
                 ^
 VACUUM (PARALLEL 2, INDEX_CLEANUP FALSE) pvactst;
 VACUUM (PARALLEL 2, FULL TRUE) pvactst; -- error, cannot use both PARALLEL and FULL
-ERROR:  cannot specify both FULL and PARALLEL options
+ERROR:  cannot specify both FULL and parallel workers
+VACUUM (PARALLEL 0, FULL TRUE) pvactst; -- can specify parallel disable (even though that's implied by FULL)
 VACUUM (PARALLEL) pvactst; -- error, cannot use PARALLEL option without parallel degree
 ERROR:  parallel option requires a value between 0 and 1024
 LINE 1: VACUUM (PARALLEL) pvactst;
                 ^
 CREATE TEMPORARY TABLE tmp (a int PRIMARY KEY);
 CREATE INDEX tmp_idx1 ON tmp (a);
-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
 RESET min_parallel_index_scan_size;
 DROP TABLE pvactst;
diff --git a/src/test/regress/sql/vacuum.sql b/src/test/regress/sql/vacuum.sql
index cf741f7b11..9743db35c7 100644
--- a/src/test/regress/sql/vacuum.sql
+++ b/src/test/regress/sql/vacuum.sql
@@ -99,10 +99,11 @@ VACUUM (PARALLEL 0) pvactst; -- disable parallel vacuum
 VACUUM (PARALLEL -1) pvactst; -- error
 VACUUM (PARALLEL 2, INDEX_CLEANUP FALSE) pvactst;
 VACUUM (PARALLEL 2, FULL TRUE) pvactst; -- error, cannot use both PARALLEL and FULL
+VACUUM (PARALLEL 0, FULL TRUE) pvactst; -- can specify parallel disable (even though that's implied by FULL)
 VACUUM (PARALLEL) pvactst; -- error, cannot use PARALLEL option without parallel degree
 CREATE TEMPORARY TABLE tmp (a int PRIMARY KEY);
 CREATE INDEX tmp_idx1 ON tmp (a);
-VACUUM (PARALLEL 1) tmp; -- disables parallel vacuum option
+VACUUM (PARALLEL 1) tmp; -- parallel vacuum disabled for temp tables
 RESET min_parallel_index_scan_size;
 DROP TABLE pvactst;
 
-- 
2.17.0


--YC8Ek3FeOE8ywfXk--