Hi, 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.
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. I added the test to pg_visibility tests because I use pg_truncate_visibility_map(). That seemed better than adding a new test module or something. It doesn't test the extension functionality specifically, but it seems like other tests in pg_visibility.sql also exercise core code. Let me know if this interpretation is off-base. - Melanie [1] https://www.postgresql.org/message-id/CAAKRu_bvkCZ%2BxBzBjujJMkA5STU%2Bb6AtrUUTjcvAH%3DZnnpTtzA%40mail.gmail.com
From fa16946d3ab986542436efa57248abaf11f77fc0 Mon Sep 17 00:00:00 2001 From: Melanie Plageman <[email protected]> Date: Wed, 10 Dec 2025 12:34:58 -0500 Subject: [PATCH v1] Test vacuum setting VM for unmodified heap buffer When setting the VM during phase I vacuum, it is possible that the heap buffer requires no modification. This can happen if the VM was truncated or destroyed. Because we still add the heap buffer to the WAL chain (we use it to update the freespace map in recovery), currently, we require that the heap buffer be marked dirty anyway. We could pass REGBUF_NO_CHANGES when registering the buffer, but that adds fiddly extra logic for an uncommon case. Either way, we had no test coverage for this, so it's best to add it. --- .../pg_visibility/expected/pg_visibility.out | 17 +++++++++++++++++ contrib/pg_visibility/sql/pg_visibility.sql | 13 +++++++++++++ 2 files changed, 30 insertions(+) diff --git a/contrib/pg_visibility/expected/pg_visibility.out b/contrib/pg_visibility/expected/pg_visibility.out index 09fa5933a35..3608f801eee 100644 --- a/contrib/pg_visibility/expected/pg_visibility.out +++ b/contrib/pg_visibility/expected/pg_visibility.out @@ -204,6 +204,23 @@ select pg_truncate_visibility_map('test_partition'); (1 row) +-- test the case where vacuum phase I does not need to modify the heap buffer +-- and only needs to set the VM +create table test_vac_unmodified_heap(a int); +insert into test_vac_unmodified_heap values (1); +vacuum (freeze) test_vac_unmodified_heap; +-- the checkpoint cleans the buffer dirtied by freezing the sole tuple +checkpoint; +-- truncating the VM ensures that the next vacuum will need to set it +select pg_truncate_visibility_map('test_vac_unmodified_heap'); + pg_truncate_visibility_map +---------------------------- + +(1 row) + +-- vacuum sets the VM but does not need to set PD_ALL_VISIBLE so no heap page +-- modification +vacuum test_vac_unmodified_heap; -- test copy freeze create table copyfreeze (a int, b char(1500)); -- load all rows via COPY FREEZE and ensure that all pages are set all-visible diff --git a/contrib/pg_visibility/sql/pg_visibility.sql b/contrib/pg_visibility/sql/pg_visibility.sql index 5af06ec5b76..6af7c179df0 100644 --- a/contrib/pg_visibility/sql/pg_visibility.sql +++ b/contrib/pg_visibility/sql/pg_visibility.sql @@ -94,6 +94,19 @@ select count(*) > 0 from pg_visibility_map_summary('test_partition'); select * from pg_check_frozen('test_partition'); -- hopefully none select pg_truncate_visibility_map('test_partition'); +-- test the case where vacuum phase I does not need to modify the heap buffer +-- and only needs to set the VM +create table test_vac_unmodified_heap(a int); +insert into test_vac_unmodified_heap values (1); +vacuum (freeze) test_vac_unmodified_heap; +-- the checkpoint cleans the buffer dirtied by freezing the sole tuple +checkpoint; +-- truncating the VM ensures that the next vacuum will need to set it +select pg_truncate_visibility_map('test_vac_unmodified_heap'); +-- vacuum sets the VM but does not need to set PD_ALL_VISIBLE so no heap page +-- modification +vacuum test_vac_unmodified_heap; + -- test copy freeze create table copyfreeze (a int, b char(1500)); -- 2.43.0
