On Thu, Apr 9, 2020 at 12:14 PM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Thu, Apr 09, 2020 at 11:05:50AM +0530, Amit Kapila wrote:
> > On Thu, Apr 9, 2020 at 7:07 AM Michael Paquier <michael@paquier.xyz> wrote:
> >> I think that
> >> this patch needs tests in sql/vacuum.sql.
> >
> > We already have one test that is testing an invalid combination of
> > PARALLEL and FULL option, not sure of adding more on similar lines is
> > a good idea, but we can do that if it makes sense. What more tests
> > you have in mind which make sense here?
>
> As you say, vacuum.sql includes this test:
> VACUUM (PARALLEL 2, FULL TRUE) pvactst; -- error, cannot use both PARALLEL and FULL
> ERROR: cannot specify both FULL and PARALLEL options
>
> But based on the discussion of this thread, it seems to me that we had
> better stress more option combinations, particularly the two following
> ones:
> vacuum (full 0, parallel 1) foo;
> vacuum (full 1, parallel 0) foo;
>
> Without that, how do you make sure that the compatibility wanted does
> not break again in the future? As of HEAD, the first one passes and
> the second one fails. And as Tushar is telling us we want to
> handle both cases in a consistent way.
>
We can add more tests to validate the syntax, but my worry was to not
increase test timing by doing (parallel) vacuum. So maybe we can do
such syntax validation on empty tables or you have any better idea?
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com