Re: Strange assertion using VACOPT_FREEZE in vacuum.c

On Sat, Feb 14, 2015 at 8:10 AM, Jim Nasby wrote:
>
> On 2/12/15 10:54 PM, Michael Paquier wrote:
>>
>> Hi all,
>>
>> When calling vacuum(), there is the following assertion using VACOPT_FREEZE:
>> Assert((vacstmt->options & VACOPT_VACUUM) ||
>> !(vacstmt->options & (VACOPT_FULL | VACOPT_FREEZE)));
>> I think that this should be changed with sanity checks based on the
>> parameter values of freeze_* in VacuumStmt as we do not set up
>> VACOPT_FREEZE when VACUUM is used without options in parenthesis, for
>> something like that:
>> Assert((vacstmt->options & VACOPT_VACUUM) ||
>> - !(vacstmt->options & (VACOPT_FULL | VACOPT_FREEZE)));
>> + ((vacstmt->options & VACOPT_FULL) == 0 &&
>> + vacstmt->freeze_min_age < 0 &&
>> + vacstmt->freeze_table_age < 0 &&
>> + vacstmt->multixact_freeze_min_age < 0 &&
>> + vacstmt->multixact_freeze_table_age < 0));
>> This would also have the advantage to limit the use of VACOPT_FREEZE
>> in the query parser.
>> A patch is attached.
>> Thoughts?
>
>
> Looks good. Should we also assert that if VACOPT_FREEZE is set then all the other stuff is 0? I don't know what kind of sanity checks we normally try and put on the parser, but that seems like a possible hole.

Possible, but this would require at least to change gram.y to update
VacuumStmt->options to set VACOPT_FREEZE for the case where VACUUM is
parsed without parenthesis. I'd rather simply rely on the freeze
parameters for simplicity. That is at least what I guess by looking at
where is used VACOPT_FREEZE.
--
Michael