Re: A few new options for vacuumdb

On Tue, Jan 29, 2019 at 09:48:18PM +0000, Bossart, Nathan wrote:
> On 1/28/19, 6:35 PM, "Michael Paquier" <michael(at)paquier(dot)xyz> wrote:
>> - " ON c.relnamespace OPERATOR(pg_catalog.=) ns.oid\n");
>> + " ON c.relnamespace OPERATOR(pg_catalog.=) ns.oid\n"
>> + " LEFT JOIN pg_catalog.pg_class t"
>> + " ON c.reltoastrelid OPERATOR(pg_catalog.=) t.oid\n");
>> Why do need this part?
>
> This is modeled after the query provided in the docs for preventing
> transaction ID wraparound [0]. I think the idea is to combine the
> relation with its TOAST table so that it does not need to be
> considered separately. The VACUUM commands generated in vacuumdb will
> also process the corresponding TOAST table for the relation, anyway.

Oh, OK. This makes sense. It would be nice to add a comment in the
patch and to document this calculation method in the docs of
vacuumdb.

> I noticed a behavior change from the catalog query patch that we
> probably ought to fix. The "WHERE c.relkind IN ('r', 'm')" clause
> seems sufficient to collect all vacuumable relations (TOAST tables are
> handled when vacuuming the main relation, and partitioned tables are
> handled by vacuuming the partitions individually), but it is not
> sufficient to match the previous behavior when --table is used.
> Previously, we did not filter by relkind at all when --table is used.
> Instead, we let the server emit a WARNING when a relation that
> couldn't be processed was specified.

Indeed, the WARNING can be useful for some users when trying to work
on an incorrect relation kind, especially when not using --verbose.
Fixed after adding a test with command_checks_all.

> Unfortunately, this complicates the --min-xid-age and --min-mxid-age
> patch a bit, as some of the relation types that can be vacuumed and/or
> analyzed do not really have a transaction ID age. AFAICT the simplest
> way to handle this case is to filter out relations with a relfrozenxid
> or relminmxid of 0.

We should be able to live with that.
--
Michael