The following review has been posted through the commitfest application:
make installcheck-world: not tested
Implements feature: not tested
Spec compliant: not tested
Documentation: not tested
I have read this patch. I like the concept and would like it to get committed.
Question I have after reading the patch is around this construct:
/*
- * If there are no indexes then we can vacuum the page right now
- * instead of doing a second scan.
+ * If there are no indexes we can vacuum the page right now instead of
+ * doing a second scan. Also we don't do that but forget dead tuples
+ * when index cleanup is disabled.
*/
This seems to change behavior on heap tuples, even though the option itself is documented to be about "Indexes" only.
Thisneeds either better explanation what "forget dead tuples" means and that it does not lead to some kind of internal
inconsistency,or documentation on what is the effect on heap tuples.
This same block raises a question on "after I enable this option, do a vacuum, decide I don't like it, what do I need
todo to disable it back?" - just set it back, or set and perform a vacuum, or set and perform a VACUUM FULL as
somethingwas "forgotten"?
It may happen this concept of "forgetting" is documented somewhere in the near comments but I'd prefer it to be stated
explicitly.
The new status of this patch is: Waiting on Author
