On 1/29/19, 4:47 PM, "Michael Paquier" <michael@paquier.xyz> wrote:
> On Tue, Jan 29, 2019 at 09:48:18PM +0000, Bossart, Nathan wrote:
>> On 1/28/19, 6:35 PM, "Michael Paquier" <michael@paquier.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.
Sure, this is added in v8.
>> 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.
Thanks. Something else I noticed is that we do not retrieve foreign
tables and partitioned tables for --analyze and --analyze-only.
However, that has long been the case for parallel mode, and this issue
should probably get its own thread.
Nathan
