While working on a patch to set the VM in the same WAL record as pruning and freezing [1], I discovered we have no test coverage of the case where vacuum phase I sets the VM but no modifications are made to the heap buffer (not even setting PD_ALL_VISIBLE). This can only happen when the VM was somehow removed or destroyed.
+1 for adding the test, but IIUC PD_ALL_VISIBLE is being set in this case during the "vacuum test_vac_unmodified_heap;" because VM bit is not set (as we truncated VM) and presult.all_visible is true as well , so it goes in if (!all_visible_according_to_vm && presult.all_visible), where its doing these, this was the flow i observed while trying to understand the patch by running the given test, please correct me if I'm wrong.
Currently, we require the heap buffer to be marked dirty even if it is unmodified because we add it to the WAL chain and do not pass REGBUF_NO_CHANGES. (And we require adding it to the WAL chain because we update the freespace map using the heap buffer in recovery). The VM being gone is an uncommon case, so I don't think it makes sense to add special logic to pass REGBUF_NO_CHANGES. However, I do think we should have a test for this case.
makes sense, i think this below comment supports your final decision of not optimizing it.
* NB: If the heap page is all-visible but the VM bit is not set, we * don't need to dirty the heap page. However, if checksums are * enabled, we do need to make sure that the heap page is dirtied * before passing it to visibilitymap_set(), because it may be logged. * Given that this situation should only happen in rare cases after a * crash, it is not worth optimizing. */
