On Mon, May 20, 2019 at 11:43:19AM +0900, Masahiko Sawada wrote:
> Thank you for comments. Attached updated version patch.
This is an open item present for quite some time, so I have looked at
the patch. The base patch is fine.
+INSERT INTO no_index_cleanup(i, t) VALUES(1, repeat('1234567890',30000));
Do we really need a string as long as that?
Is actually the existing set of tests that helpful? We now have only
two VACUUM queries which run on no_index_cleanup, both of them using
FULL so the reloption as well as the value of INDEX_CLEANUP are
ignored. Wouldn't it be better to redesign a bit the tests with more
combinations of options like that?
-- Toast inherit the value from the table
ALTER TABLE no_index_cleanup SET (vacuum_index_cleanup = false);
-- Value directly defined for both
ALTER TABLE no_index_cleanup SET (vacuum_index_cleanup = false,
toast.vacuum_index_cleanup = true);
ALTER TABLE no_index_cleanup SET (vacuum_index_cleanup = true,
toast.vacuum_index_cleanup = false);
It seems to me that we'd want tests to make sure that indexes are
actually cleaned up, where pageinspect could prove to be useful.
--
Michael