From: Justin Pryzby <pryzby@telsasoft.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
Cc: Mahendra Singh Thalor <mahi6run@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: tushar <tushar.ahuja@enterprisedb.com>
Cc: pgsql-hackers@postgresql.org
Subject: Re: Vacuum o/p with (full 1, parallel 0) option throwing an error
Date: Fri, 10 Apr 2020 08:35:32 -0500
Message-ID: <20200410133532.GA2228@telsasoft.com> (raw)
In-Reply-To: <CAA4eK1Ku4X-RYcT+h=ngE8Yt6fMCD2rXpAvG0rh=j=PS+K0zmg@mail.gmail.com>
References: <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>
<20200409083319.GN2228@telsasoft.com>
<CAA4eK1Ku4X-RYcT+h=ngE8Yt6fMCD2rXpAvG0rh=j=PS+K0zmg@mail.gmail.com>
On Fri, Apr 10, 2020 at 10:34:02AM +0530, Amit Kapila wrote:
> On Thu, Apr 9, 2020 at 2:03 PM Justin Pryzby <pryzby@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
Attachments:
[text/x-diff] v3-0001-Allow-specifying-parallel-0-with-vacuum-full.patch (4.1K, ../20200410133532.GA2228@telsasoft.com/2-v3-0001-Allow-specifying-parallel-0-with-vacuum-full.patch)
download | inline diff:
From 16ff7441791c481ab97b7eb8b2948e38626c7273 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@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.cindex 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.outindex 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.sqlindex 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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: pryzby@telsasoft.com, amit.kapila16@gmail.com, masahiko.sawada@2ndquadrant.com, mahi6run@gmail.com, robertmhaas@gmail.com, tushar.ahuja@enterprisedb.com
Subject: Re: Vacuum o/p with (full 1, parallel 0) option throwing an error
In-Reply-To: <20200410133532.GA2228@telsasoft.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox