On Fri, Apr 10, 2020 at 10:34:02AM +0530, Amit Kapila wrote: > On Thu, Apr 9, 2020 at 2:03 PM Justin Pryzby <pry...@telsasoft.com> wrote: > > 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. > > > > Okay, but I think the error message suggested by Robert "ERROR: VACUUM > FULL cannot be performed in parallel" sounds better than what you have > proposed. What do you think?
No problem. I think I was trying to make my text similar to that from 14a4f6f37. I realized that I didn't sq!uash my last patch, so it didn't include the functional change (which is maybe what Robert was referring to). -- Justin
>From 16ff7441791c481ab97b7eb8b2948e38626c7273 Mon Sep 17 00:00:00 2001 From: Justin Pryzby <pryz...@telsasoft.com> Date: Wed, 8 Apr 2020 11:38:36 -0500 Subject: [PATCH v3] Allow specifying "(parallel 0)" with "vacuum(full)".. ..even though full implies parallel 0 Check options using the same test as in vacuumlazy.c See also similar change long ago: 14a4f6f3748df4ff63bb2d2d01146b2b98df20ef Discussion: https://www.postgresql.org/message-id/58c8d171-e665-6fa3-a9d3-d9423b694dae%40enterprisedb.com --- src/backend/commands/vacuum.c | 6 ++---- src/test/regress/expected/vacuum.out | 5 +++-- src/test/regress/sql/vacuum.sql | 3 ++- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index 351d5215a9..9cb65d76a9 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,10 +197,10 @@ 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"))); + errmsg("VACUUM FULL cannot be performed in parallel"))); /* * 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..f90c498813 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: VACUUM FULL cannot be performed in parallel +VACUUM (PARALLEL 0, FULL TRUE) pvactst; -- can specify parallel disabled (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..dade1e5e57 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 disabled (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
