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.

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

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




Attachment: v1-0001-Fix-inaccurate-LSN-validation-messages-in-pg_wali.patch
Description: Binary data

Attachment: v1-0002-Handle-empty-LSN-ranges-in-pg_walinspect.patch
Description: Binary data

Reply via email to