Re: Strange assertion using VACOPT_FREEZE in vacuum.c

Michael Paquier wrote:

> 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);

Here's an updated patch.  I took your latest version and made some extra
changes:

1. ordered the argument list to vacuum(), hopefully it's more sensible
now.

2. changed struct autovac_table so that it uses "options" (the same
VacuumOption bitmask to be passed to vacuum) and VacuumParams, instead
of having each struct member separately.  That way, the parameters to
vacuum() are constructed at once in autovac_recheck_table, and
autovacuum_do_vac_analyze becomes much simpler.

3. Added VACOPT_SKIPTOAST to VacuumOptions, currently only used by
autovacuum.  We remove the do_toast argument.

I think this is pretty sensible and my inclination is to commit as is,
so that we can finally move on to more interesting things (such as the
new reloption being proposed in a nearby thread).

--
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services