Re: Strange assertion using VACOPT_FREEZE in vacuum.c

On Thu, Mar 12, 2015 at 4:14 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Wed, Mar 11, 2015 at 3:09 PM, Alvaro Herrera
> <alvherre@2ndquadrant.com> wrote:
>> Robert Haas wrote:
>>> On Fri, Mar 6, 2015 at 1:39 AM, Michael Paquier
>>> <michael.paquier@gmail.com> wrote:
>>
>>> > - 0001 is the previous one
>>> > - 0002 removes VacuumStmt from the call stack of ANALYZE and VACUUM routines
>>> > - 0003 moves for_wraparound in VacuumParams.
>>>
>>> Yeah, I think something like this could be a sensible approach.
>>
>> But autovacuum is still manufacturing a VacuumStmt by hand.  If we want
>> to get rid of that, I think it'd work to have a new
>> ExecVacuum(VacuumStmt, params) function which is called from
>> standard_ProcessUtility and does just vacuum(rel, relid, params).
>> Autovacuum on the other hand can call vacuum() without having to
>> construct the parse node.
>
> +1.

I have been pondering about that, and code-speaking this gives the
attached, making VacuumStmt only be used when VACUUM/ANALYZE is kicked
through utility.c, and removing its dependency in autovacuum.c.

There are a couple of parameters like va_cols, bstrategy, do_toast or
relid that can change depending on the code path (like presence of
toast relation). I think that it is confusing to add them in
VacuumParams as their value would get duplicated in this structure and
in the modified values that need to be set, so this structure only
contains the freeze control parameters and for_wraparound.

In utility.c, the interface for VACUUM/ANALYZE has this shape:
void ExecVacuum(VacuumStmt *vacstmt, bool isTopLevel);
--
Michael