On Thu, 9 Apr 2020 at 14:52, Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Thu, Apr 9, 2020 at 12:09 AM Justin Pryzby <pry...@telsasoft.com> wrote: > > > > On Thu, Apr 09, 2020 at 12:06:04AM +0530, Mahendra Singh Thalor wrote: > > > On Wed, 8 Apr 2020 at 22:11, Justin Pryzby <pry...@telsasoft.com> wrote: > > > > > > > > > > Thanks Justin for the patch. > > > > > > Patch looks fine to me and it is fixing the issue. After applying this > > > patch, vacuum will work as: > > > > > > 1) vacuum (parallel 1, full 0); > > > -- vacuuming will be done with 1 parallel worker. > > > 2) vacuum (parallel 0, full 1); > > > -- full vacuuming will be done. > > > 3) vacuum (parallel 1, full 1); > > > -- will give error :ERROR: cannot specify both FULL and PARALLEL options > > > > > > 3rd example is telling that we can't give both FULL and PARALLEL > > > options but in 1st and 2nd, we are giving both FULL and PARALLEL > > > options and we are not giving any error. I think, irrespective of > > > value of both FULL and PARALLEL options, we should give error in all > > > the above mentioned three cases. > > > > I think the behavior is correct, but the error message could be improved, > > > > Yeah, I also think that the behavior is fine.
Me too. > We can do what Mahendra > is saying but that will unnecessarily block some syntax and we might > need to introduce an extra variable to detect that in code. ISTM we have been using the expression "specifying the option" in log messages when a user wrote the option in the command. But now that VACUUM command options can have a true/false it doesn't make sense to say like "if the option is specified we cannot do that". So maybe "cannot turn on FULL and PARALLEL options" or something would be better, I think. Regards, -- Masahiko Sawada http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
