On Tue, 2009-12-01 at 01:43 -0800, Jeff Davis wrote:
> Marking as ready.
You're saying its ready, yet there are 3 additional suggestions patches
attached here. Please can you resolve these and re-submit a single final
patch from author and reviewer?
I will review and eventually commit this, if appropriate. It is either
1st or 2nd in my queue, unless someone else grabs it first.
Review comments
* What happens if you issue VACUUM FULL; which we would expect to use
the new method of vacuum on all tables in the database. Won't that just
fail with an error when it comes to catalog tables? Sounds to me like we
should avoid the error and just silently do an INPLACE on catalog
tables.
* Such a pivotal change to Postgres needs more test coverage than a
single line in regression tests. It might have been OK before, but I
think we need a few more combinations here, at least in this release:
with, without indexes, empty table, clustered, non-clustered etc and of
course a full database VACUUM so that we have the catalog table case
covered, plus an explicit catalog table vacuum.
-- Simon Riggs www.2ndQuadrant.com
