On Wed, Jan 30, 2019 at 05:45:58PM +0000, Bossart, Nathan wrote:
> On 1/29/19, 4:47 PM, "Michael Paquier" <michael@paquier.xyz> wrote:
>> 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.
Thanks, Nathan.
Something which was not correct in the patch is the compatibility of
the query. xid <> xid has been added in 9.6, so the new options will
not be able to work with older versions. The versions marked as
compatible in the last patch came from the age-ing functions, but you
added direct comparisons with relfrozenxid and relminmxid in the
latest versions of the patch. This implementation goes down a couple
of released versions, which is useful enough in my opinion, so I would
keep it as-is.
I have added as well some markups around "PostgreSQL" in the docs, and
extra casts for the integer/xid values of the query. The test
patterns are also simplified, and I added tests for incorrect values
of --min-xid-age and --min-mxid-age. Does that look correct to you?
> 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.
Good point, this goes down a couple of releases, and statistics on
both may be useful to compile for a system-wide operation. Spawning a
separate thread looks adapted to me.
--
Michael