Thanks for review! Jeff Davis <pg...@j-davis.com> wrote:
> * "VACUUM (FULL REPLACE) pg_class" should be rejected, not silently > turned into "VACUUM (FULL INPLACE) pg_class". Hmmm, it requires to remember whether REPLACE is specified or not for the non-INPLACE vacuum, but I don't want to add VACOPT_REPLACE only for the purpose. I just removed "FULL REPLACE" syntax; Only "FULL" and "FULL INPLACE" are available. VACUUM FULL without INPLACE behaves as cluster-like rewrites for non-system tables. FULL INPLACE is a traditional vacuum full. System catalogs are always vacuumed with INPLACE version. - VACUUM FULL / VACUUM (FULL) => rewritten version - VACUUM (FULL INPLACE) => traditional version > * In vacuumdb, the code for handling INPLACE is a little ugly. Perhaps > it should be changed to always use your new options syntax? That might > be more code, but it would be a little more readable. It might make the code cleaner, but I want vacuumdb in 8.5 to work on older versions of servers unless we use the new feature. Older servers can only accept older syntax, so I avoided using the new syntax if not needed. (The new patch still uses two versions of syntax.) > * The patch should be merged with CVS HEAD. The changes required are > minor; but notice that there is a minor style difference in the assert > in vacuum(). > * vacuumdb should reject -i without -f > * The replace or inplace option is a "magical" default, because "VACUUM > FULL" defaults to "replace" for user tables and "inplace" for system > tables. I tried to make that more clear in my documentation suggestions. > * There are some windows line endings in the patch, which should be > removed. Done. Regards, --- ITAGAKI Takahiro NTT Open Source Software Center
vacuum-full_20091130.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
