Re: [Proposal] Allow users to specify multiple tables in VACUUM commands

On Fri, Sep 22, 2017 at 7:26 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> "Bossart, Nathan" <bossartn(at)amazon(dot)com> writes:
>> [ 0001-error_on_duplicate_columns_in_analyze_v6.patch ]
>
> I've pushed (and back-patched) the 0001 patch, with some significant
> changes:

Thanks. Arrived here too late to give feedback on the proposed patch.

> I'll take a look at the updated 0002 tomorrow, but if anyone living in
> a different timezone wants to review it meanwhile, feel free.

Here is some feedback. 0002 fails to apply after 0001 has been
committed in its regression tests, that can easily be resolved.

A couple of tests could be removed:
+VACUUM (FREEZE, ANALYZE) vactst (i);
+VACUUM (FREEZE, ANALYZE) vactst (i, i, i);
+VACUUM vactst (i);

void
-vacuum(int options, RangeVar *relation, Oid relid, VacuumParams *params,
- List *va_cols, BufferAccessStrategy bstrategy, bool isTopLevel)
+vacuum(int options, VacuumRelation *relation, VacuumParams *params,
+ BufferAccessStrategy bstrategy, bool isTopLevel)
[...]
- relations = get_rel_oids(relid, relation);
+ relations = get_rel_oids(relation);
I still think that ExecVacuum() should pass a list of VacuumRelation
objects to vacuum(), and get_rel_oids() should take in input a list,
and return a completed lists. This way the decision-making of doing
everything in the same transaction should happens once in vacuum().
And actually, if several relations are defined with VACUUM, your patch
would not use one transaction per table as use_own_xacts would be set
to false. I think that Tom meant that relations whose processed has
finished have to be committed immediately. Per your patch, the commit
happens once all relations are committed.
--
Michael