Re: Strange assertion using VACOPT_FREEZE in vacuum.c - Mailing list pgsql-hackers

From Peter Eisentraut
Subject Re: Strange assertion using VACOPT_FREEZE in vacuum.c
Date
Msg-id 54E64A82.3000500@gmx.net
Whole thread Raw
In response to Re: Strange assertion using VACOPT_FREEZE in vacuum.c  (Michael Paquier <michael.paquier@gmail.com>)
Responses Re: Strange assertion using VACOPT_FREEZE in vacuum.c
List pgsql-hackers
On 2/18/15 1:26 AM, Michael Paquier wrote:
> On Wed, Feb 18, 2015 at 10:06 AM, Michael Paquier wrote:
>> Yes, the existing assertion is right. My point is that it is strange
>> that we do not check the values of freeze parameters for an ANALYZE
>> query, which should be set to -1 all the time. If this is thought as
>> not worth checking, I'll drop this patch and my concerns.
> 
> Perhaps this explains better what I got in mind, aka making the
> assertion stricter:
>         Assert((vacstmt->options & VACOPT_VACUUM) ||
> -                  !(vacstmt->options & (VACOPT_FULL | VACOPT_FREEZE)));
> +                  ((vacstmt->options & (VACOPT_FULL | VACOPT_FREEZE)) == 0 &&
> +                       vacstmt->freeze_min_age < 0 &&
> +                       vacstmt->freeze_table_age < 0 &&
> +                       vacstmt->multixact_freeze_min_age < 0 &&
> +                       vacstmt->multixact_freeze_table_age < 0));

That's cool if you want to add those assertions, but please make them
separate statements each, like

Assert(vacstmt->options & (VACOPT_FULL | VACOPT_FREEZE) || vacstmt->freeze_min_age == -1);
Assert(vacstmt->options & (VACOPT_FULL | VACOPT_FREEZE) || vacstmt->freeze_table_age == -1);
...

Besides being more readable, this will give you more useful output if
the assertion fails.




pgsql-hackers by date:

Previous
From: Peter Eisentraut
Date:
Subject: Re: pg_dump gets attributes from tables in extensions
Next
From: Peter Eisentraut
Date:
Subject: Re: Enforce creation of destination folders for source files in pg_regress (Was: pg_regress writes into source tree)