Re: pg_walinspect: fix LSN validation messages and empty range handling

> On Sep 21, 2026, at 02:40, Kiran Kaki <itskkpg(at)gmail(dot)com> wrote:
>
> Hi Chao
>
> On Sat, Sep 19, 2026 at 8:08 PM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>>
>> Hi,
>>
>> While working on patch [1], I noticed two small issues with pg_walinspect.
>>
>> 1. In pg_get_wal_record_info() as well as a few other functions, there are checks like:
>> ```
>> if (lsn > curr_lsn)
>> ereport(ERROR,
>> (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
>> errmsg("WAL input LSN must be less than current LSN"),
>> errdetail("Current WAL LSN on the database system is at %X/%08X.",
>> LSN_FORMAT_ARGS(curr_lsn))));
>> ```
>>
>> The check itself uses a > comparison, so equality is accepted by this validation check. However, the error message says "must be less than", which implies that equality is not accepted. Thus, the check and the error message are inconsistent.
>>
>> Commit 5c1b6628075a changed the check from >= to > and changed the error message from "cannot accept future input LSN" to "WAL input LSN must be less than current LSN". This seems to have been an oversight.
>>
>> The error message can be changed to say "must be less than or equal to", matching the actual validation condition.
>
> Thanks for the patch. 0001 looks good to me.

Thank you very much for the review.

>
> As I see 5c1b6628075a relaxed these checks from >= to > but left the messages
> saying "less than", so the wording no longer matches the code. I
> checked the thread behind that commit to confirm accepting equality
> was deliberate.
>
> Tested on master (9e17d25e79d4):
>
> - Applies cleanly, builds with no new warnings
> - pg_walinspect/regress passes, and so does the full suite
> - Expected-output changes match the new messages exactly
> - No other LSN validation message has the same problem
>
> Also, the existing tests already cover every message that changed.
>
>
>> 2. pg_get_wal_records_info() accepts an end_lsn equal to start_lsn, but the same fixed range can produce different results. For example:
>> ```
>> evantest=# select pg_current_wal_flush_lsn();
>> pg_current_wal_flush_lsn
>> --------------------------
>> 0/01D61428
>> (1 row)
>> evantest=# SELECT * FROM pg_get_wal_records_info('0/01D61428', '0/01D61428');
>> ERROR: could not find a valid record after 0/01D61428
>>
>> evantest=# checkpoint;
>> CHECKPOINT
>> evantest=# SELECT * FROM pg_get_wal_records_info('0/01D61428', '0/01D61428');
>> start_lsn | end_lsn | prev_lsn | xid | resource_manager | record_type | record_length | main_data_length | fpi_length | description | block_ref
>> -----------+---------+----------+-----+------------------+-------------+---------------+------------------+------------+-------------+-----------
>> (0 rows)
>> ```
>>
>> When I passed the current flushed LSN to pg_get_wal_records_info() as both start_lsn and end_lsn, it raised an error because no record was available at or after that LSN. After I ran CHECKPOINT to generate more WAL records, the same query returned zero rows.
>>
>> Thus, the same fixed range can either raise an error or return zero rows depending on whether WAL exists after end_lsn, even though WAL after end_lsn cannot belong to the requested range. This may confuse users.
>>
>> To fix, I think an empty LSN range cannot contain a complete WAL record, so it can be handled without initializing a WAL reader. So that, the record and block information functions can return zero rows, while pg_get_wal_stats() can preserve its zero-valued aggregate output.
>>
>> [1] https://www.postgresql.org/message-id/80E9F0AD-CFC5-4BE5-81DE-D8FE35E10A1C%40gmail.com
>
> 0002 looks right to me with two suggestions
>
> I reproduced the problem on an unpatched build: an empty range at the
> end of WAL errors with "could not find a valid record after * ",
> while the same empty range mid-WAL returns 0 rows. So the result
> depends on whether anything wrote WAL afterwards, which is worth
> fixing.
>
> Tested on master (9e17d25e79d4):
>
> - Applies cleanly on top of 0001, builds with no new warnings
> - pg_walinspect/regress passes, and so does the full suite
> - Empty ranges now return 0 rows instead of erroring;
> - Non-empty ranges are unaffected
>
> Two small things:
>
> 1. Minor: In GetWalStats() the "An empty range cannot contain any WAL
> records" comment sits above "if (start_lsn < end_lsn)", the
> opposite sense from the other two sites. The structure has to
> differ there since it still calls GetXLogSummaryStats(), but the
> comment reads the wrong way round. Maybe "Read records only if the
> range is non-empty."

Accepted.

>
> 2. One thing: pg_get_wal_record_info() isn't covered. Passing it the
> current LSN still errors in the reader. Same "validation accepts it,
> then the reader fails" shape, though the right answer is less clear
> there: it returns a single row rather than a set, so returning nothing
> isn't an option, and the error isn't inaccurate. I think it would be worth
> handling alongside this, if you agree the shape is the same..
>

I don’t think we should change pg_get_wal_record_info(). Its doc says:
```
<para>
If <replaceable>in_lsn</replaceable> isn't at the start of a WAL
record, information about the next valid WAL record is shown
instead. If there is no next valid WAL record, the function
raises an error.
</para>
```

The doc predates 5c1b6628075a, so I don’t think we should change the doc behavior.

However, your comment led me to notice that the documentation for pg_get_wal_records_info() says:
```
<para>
The function raises an error if
<replaceable>start_lsn</replaceable> is not available.
</para>
```

This makes the issue trickier. Before 5c1b6628075a, start_lsn had to be less than end_lsn, so an empty range was not accepted and this inconsistency could not arise. Based on the current doc, however, it seems reasonable for start_lsn == end_lsn to return zero rows when start_lsn is available and raise an error when it is not.

From this perspective, I think I should withdraw 0002.

PFA v2:

* 0001 is unchanged
* Withdraw 0002

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/