Re: Strange assertion using VACOPT_FREEZE in vacuum.c

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.