pg_wal_summary_contents() and pg_walsummary may return different results on the same WAL summary file

[Fujii Masao]

Hi,

I found that pg_wal_summary_contents() may miss some results that pg_walsummary 
returns for the same WAL summary file. Here are the steps to reproduce the 
issue:

-----------------------------
initdb -D data
echo "summarize_wal = on" >> data/postgresql.conf
pg_ctl -D data start
psql <<EOF
CREATE TABLE t AS SELECT n i, n j FROM generate_series(1, 1000) n;
DELETE FROM t;
CHECKPOINT;
VACUUM t;
CHECKPOINT;
SELECT foo.* FROM (SELECT * FROM pg_available_wal_summaries() ORDER BY 
start_lsn DESC LIMIT 1) JOIN LATERAL pg_wal_summary_contents(tli, start_lsn, 
end_lsn) foo ON true;
EOF
pg_walsummary -i data/pg_wal/summaries/$(ls -1 data/pg_wal/summaries/ | tail -1)
-----------------------------

In my test, pg_walsummary returned three records:

TS 1663, DB 5, REL 1259, FORK main: block 0
TS 1663, DB 5, REL 16384, FORK main: limit 0
TS 1663, DB 5, REL 16384, FORK vm: limit 0

However, pg_wal_summary_contents() returned only one record:

 relfilenode | reltablespace | reldatabase | relforknumber | relblocknumber | 
is_limit_block
-------------+---------------+-------------+---------------+----------------+----------------
        1259 |          1663 |           5 |             0 |              0 | f

pg_wal_summary_contents() seems to miss the summary information with "limit" 
that pg_walsummary reports. This appears to be a bug. The attached patch fixes this.

By the way, pg_wal_summary_contents() and pg_walsummary perform nearly the same 
task but are implemented in different functions. This could be the root of 
issues like this. In the future, it would be better to have a common function 
for outputting the WAL summary file that both can use.

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
From 79a1197ea3cdc69b7dda21287e612b0c9f532b55 Mon Sep 17 00:00:00 2001
From: Fujii Masao <fu...@postgresql.org>
Date: Wed, 3 Jul 2024 15:50:30 +0900
Subject: [PATCH v1] Fix bug in pg_wal_summary_contents().

Previously, pg_wal_summary_contents() might fail to report rows with
is_limit_block = true, leading to discrepancies between
pg_wal_summary_contents() and the pg_walsummary command
on the same WAL summary file. This commit resolves the issue.
---
 src/backend/backup/walsummaryfuncs.c | 34 ++++++++++++++--------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/src/backend/backup/walsummaryfuncs.c 
b/src/backend/backup/walsummaryfuncs.c
index f082488b33..7db7f131ce 100644
--- a/src/backend/backup/walsummaryfuncs.c
+++ b/src/backend/backup/walsummaryfuncs.c
@@ -118,6 +118,23 @@ pg_wal_summary_contents(PG_FUNCTION_ARGS)
                values[2] = ObjectIdGetDatum(rlocator.dbOid);
                values[3] = Int16GetDatum((int16) forknum);
 
+               /*
+                * If the limit block is not InvalidBlockNumber, emit an extra 
row
+                * with that block number and limit_block = true.
+                *
+                * There is no point in doing this when the limit_block is
+                * InvalidBlockNumber, because no block with that number or any
+                * higher number can ever exist.
+                */
+               if (BlockNumberIsValid(limit_block))
+               {
+                       values[4] = Int64GetDatum((int64) limit_block);
+                       values[5] = BoolGetDatum(true);
+
+                       tuple = heap_form_tuple(rsi->setDesc, values, nulls);
+                       tuplestore_puttuple(rsi->setResult, tuple);
+               }
+
                /* Loop over blocks within the current relation fork. */
                while (1)
                {
@@ -143,23 +160,6 @@ pg_wal_summary_contents(PG_FUNCTION_ARGS)
                                tuple = heap_form_tuple(rsi->setDesc, values, 
nulls);
                                tuplestore_puttuple(rsi->setResult, tuple);
                        }
-
-                       /*
-                        * If the limit block is not InvalidBlockNumber, emit 
an extra row
-                        * with that block number and limit_block = true.
-                        *
-                        * There is no point in doing this when the limit_block 
is
-                        * InvalidBlockNumber, because no block with that 
number or any
-                        * higher number can ever exist.
-                        */
-                       if (BlockNumberIsValid(limit_block))
-                       {
-                               values[4] = Int64GetDatum((int64) limit_block);
-                               values[5] = BoolGetDatum(true);
-
-                               tuple = heap_form_tuple(rsi->setDesc, values, 
nulls);
-                               tuplestore_puttuple(rsi->setResult, tuple);
-                       }
                }
        }
 
-- 
2.45.1