On Wed, Mar 13, 2019 at 1:56 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> I don't have a strong opinion but the using a Node would be more
> suitable in the future when we add more options to vacuum. And it
> seems to me that it's unlikely to change a Node to a plain struct. So
> there is an idea of doing it now anyway if we might need to do it
> someday.
I just tried to apply 0001 again and noticed a conflict in the
autovac_table structure in postmaster.c.
That conflict got me thinking: aren't parameters and options an awful
lot alike? Why do we need to pass around a VacuumOptions structure
*and* a VacuumParams structure to all of these functions? Couldn't we
just have one? That led to the attached patch, which just gets rid of
the separate options flag and folds it into VacuumParams. If we took
this approach, the degree of parallelism would just be another thing
that would get added to VacuumParams, and VacuumOptions wouldn't end
up existing at all.
This patch does not address the question of what the *parse tree*
representation of the PARALLEL option should look like; the idea would
be that ExecVacuum() would need to extra the value for that option and
put it into VacuumParams just as it already does for various other
things in VacuumParams. Maybe the most natural approach would be to
convert the grammar productions for the VACUUM options list so that
they just build a list of DefElems, and then have ExecVacuum() iterate
over that list and make sense of it, as for example ExplainQuery()
already does.
I kinda like the idea of doing it that way, but then I came up with
it, so maybe you or others will think it's terrible.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company