Re: Strange assertion using VACOPT_FREEZE in vacuum.c

On Sat, Feb 28, 2015 at 10:09 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
>> [nitpicking]We could improve things on both sides, aka change gram.y
>> to set VACOPT_FREEZE correctly, and add some assertions with the
>> params freeze_* at the beginning of vacuum().[/]
>
> The other issue that I have with this is that most production operations
> don't run with Asserts enabled, so if there is an actual issue here, we
> shouldn't be using Asserts to check but regular conditionals and
> throwing ereport()'s.

The only reason why I think they should be improved is for extension
developers, a simple example being a bgworker that directly calls
vacuum with a custom VacuumStmt, at least with those assertions we
could get some directions to the developer that what he is doing is
not consistent with what the code expects.

> Another approach might be to change VACOPT_FREEZE to actually be used by
> vacuum() instead of having to set 4 variables when it's not passed in.
> Perhaps we would actually take those parameters out of VacuumStmt, add a
> new argument to vacuum() for autovacuum to use which is a struct
> containing those options, and remove all of nonsense of setting those
> variables inside gram.y. At least in my head, that'd be a lot cleaner-
> have the grammar worry about options that are actually coming from the
> grammar and give other callers a way to specify more options if they've
> got them.

Hm, why not. That would remove all inconsistencies between the parser
and the autovacuum code path. Perhaps something like the attached
makes sense then?
--
Michael