On Wed, Mar 13, 2019 at 1:56 AM Masahiko Sawada <sawada.m...@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
vacuum-options-into-params.patch
Description: Binary data