pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedpg_walinspect: fix LSN validation messages and empty range handling
16+ messages / 5 participants
[nested] [flat]
* pg_walinspect: fix LSN validation messages and empty range handling
@ 2026-09-20 03:08 Chao Li <li.evan.chao@gmail.com>
0 siblings, 2 replies; 16+ messages in thread
From: Chao Li @ 2026-09-20 03:08 UTC (permalink / raw)
To: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; +Cc: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
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/
Attachments: download all (2 files, .tar.gz)
v1-0001-Fix-inaccurate-LSN-validation-messages-in-pg_wali.patch (application/octet-stream, 6.0K) download
From 30af754d2da46b0bc2af7f6805e529d4d30dac6b Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <lic@highgo.com>
Date: Sun, 20 Sep 2026 09:39:56 +0800
Subject: [PATCH v1 1/2] Fix inaccurate LSN validation messages in
pg_walinspect
Commit 5c1b6628075a changed the LSN validation checks to allow an input
LSN to equal the current LSN and a start LSN to equal the end LSN.
However, the corresponding error messages continued to say that the
LSNs had to be less than those bounds.
This was an oversight in 5c1b6628. Adjust the messages to say "less
than or equal to", matching the actual validation conditions.
Author: Chao Li <lic@highgo.com>
---
contrib/pg_walinspect/expected/oldextversions.out | 4 ++--
contrib/pg_walinspect/expected/pg_walinspect.out | 14 +++++++-------
contrib/pg_walinspect/pg_walinspect.c | 10 +++++-----
3 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/contrib/pg_walinspect/expected/oldextversions.out b/contrib/pg_walinspect/expected/oldextversions.out
index 89953af3d0e..fd65ed149c6 100644
--- a/contrib/pg_walinspect/expected/oldextversions.out
+++ b/contrib/pg_walinspect/expected/oldextversions.out
@@ -43,9 +43,9 @@ SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_stats_till_end_of_wal(:'wal_lsn1');
-- Failures with start LSNs.
SELECT * FROM pg_get_wal_records_info_till_end_of_wal('FFFFFFFF/FFFFFFFF');
-ERROR: WAL start LSN must be less than current LSN
+ERROR: WAL start LSN must be less than or equal to current LSN
SELECT * FROM pg_get_wal_stats_till_end_of_wal('FFFFFFFF/FFFFFFFF');
-ERROR: WAL start LSN must be less than current LSN
+ERROR: WAL start LSN must be less than or equal to current LSN
-- Move to new version 1.1.
ALTER EXTENSION pg_walinspect UPDATE TO '1.1';
-- List what version 1.1 contains.
diff --git a/contrib/pg_walinspect/expected/pg_walinspect.out b/contrib/pg_walinspect/expected/pg_walinspect.out
index f955ff5d3c5..b9abeeebdfd 100644
--- a/contrib/pg_walinspect/expected/pg_walinspect.out
+++ b/contrib/pg_walinspect/expected/pg_walinspect.out
@@ -29,14 +29,14 @@ SELECT * FROM pg_get_wal_block_info('0/0', :'wal_lsn1');
ERROR: could not read WAL at LSN 0/00000000
-- Start LSN > End LSN.
SELECT * FROM pg_get_wal_records_info(:'wal_lsn2', :'wal_lsn1');
-ERROR: WAL start LSN must be less than end LSN
+ERROR: WAL start LSN must be less than or equal to end LSN
SELECT * FROM pg_get_wal_stats(:'wal_lsn2', :'wal_lsn1');
-ERROR: WAL start LSN must be less than end LSN
+ERROR: WAL start LSN must be less than or equal to end LSN
SELECT * FROM pg_get_wal_block_info(:'wal_lsn2', :'wal_lsn1');
-ERROR: WAL start LSN must be less than end LSN
+ERROR: WAL start LSN must be less than or equal to end LSN
-- LSNs with the highest value possible.
SELECT * FROM pg_get_wal_record_info('FFFFFFFF/FFFFFFFF');
-ERROR: WAL input LSN must be less than current LSN
+ERROR: WAL input LSN must be less than or equal to current LSN
-- Success with end LSNs.
SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_records_info(:'wal_lsn1', 'FFFFFFFF/FFFFFFFF');
ok
@@ -58,11 +58,11 @@ SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_block_info(:'wal_lsn1', 'FFFFFFFF/FFF
-- Failures with start LSNs.
SELECT * FROM pg_get_wal_records_info('FFFFFFFF/FFFFFFFE', 'FFFFFFFF/FFFFFFFF');
-ERROR: WAL start LSN must be less than current LSN
+ERROR: WAL start LSN must be less than or equal to current LSN
SELECT * FROM pg_get_wal_stats('FFFFFFFF/FFFFFFFE', 'FFFFFFFF/FFFFFFFF');
-ERROR: WAL start LSN must be less than current LSN
+ERROR: WAL start LSN must be less than or equal to current LSN
SELECT * FROM pg_get_wal_block_info('FFFFFFFF/FFFFFFFE', 'FFFFFFFF/FFFFFFFF');
-ERROR: WAL start LSN must be less than current LSN
+ERROR: WAL start LSN must be less than or equal to current LSN
-- ===================================================================
-- Tests for all function executions
-- ===================================================================
diff --git a/contrib/pg_walinspect/pg_walinspect.c b/contrib/pg_walinspect/pg_walinspect.c
index a172f9e2b40..6edd6f8cb35 100644
--- a/contrib/pg_walinspect/pg_walinspect.c
+++ b/contrib/pg_walinspect/pg_walinspect.c
@@ -488,7 +488,7 @@ pg_get_wal_record_info(PG_FUNCTION_ARGS)
if (lsn > curr_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("WAL input LSN must be less than current LSN"),
+ errmsg("WAL input LSN must be less than or equal to current LSN"),
errdetail("Current WAL LSN on the database system is at %X/%08X.",
LSN_FORMAT_ARGS(curr_lsn))));
@@ -530,14 +530,14 @@ ValidateInputLSNs(XLogRecPtr start_lsn, XLogRecPtr *end_lsn)
if (start_lsn > curr_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("WAL start LSN must be less than current LSN"),
+ errmsg("WAL start LSN must be less than or equal to current LSN"),
errdetail("Current WAL LSN on the database system is at %X/%08X.",
LSN_FORMAT_ARGS(curr_lsn))));
if (start_lsn > *end_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("WAL start LSN must be less than end LSN")));
+ errmsg("WAL start LSN must be less than or equal to end LSN")));
if (*end_lsn > curr_lsn)
*end_lsn = curr_lsn;
@@ -836,7 +836,7 @@ pg_get_wal_records_info_till_end_of_wal(PG_FUNCTION_ARGS)
if (start_lsn > end_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("WAL start LSN must be less than current LSN"),
+ errmsg("WAL start LSN must be less than or equal to current LSN"),
errdetail("Current WAL LSN on the database system is at %X/%08X.",
LSN_FORMAT_ARGS(end_lsn))));
@@ -855,7 +855,7 @@ pg_get_wal_stats_till_end_of_wal(PG_FUNCTION_ARGS)
if (start_lsn > end_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("WAL start LSN must be less than current LSN"),
+ errmsg("WAL start LSN must be less than or equal to current LSN"),
errdetail("Current WAL LSN on the database system is at %X/%08X.",
LSN_FORMAT_ARGS(end_lsn))));
--
2.50.1 (Apple Git-155)
=
v1-0002-Handle-empty-LSN-ranges-in-pg_walinspect.patch (application/octet-stream, 4.9K) download
From b3e48f4724c1db83ef1ee1b9b9cbd413d0075a0d Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <lic@highgo.com>
Date: Sun, 20 Sep 2026 10:45:27 +0800
Subject: [PATCH v1 2/2] Handle empty LSN ranges in pg_walinspect
The pg_walinspect range functions accept equal start and end LSNs.
Such a range cannot contain a complete WAL record, but the functions
still initialized a WAL reader and searched for the next valid record.
Consequently, an empty range could either raise an error or return an
empty result depending on whether later WAL was available.
Skip WAL reader initialization for empty ranges. Return no rows from
the record and block information functions, while preserving the
existing zero-valued aggregate output from pg_get_wal_stats().
Author: Chao Li <lic@highgo.com>
---
.../pg_walinspect/expected/pg_walinspect.out | 27 +++++++++++++++++
contrib/pg_walinspect/pg_walinspect.c | 30 +++++++++++++------
contrib/pg_walinspect/sql/pg_walinspect.sql | 8 +++++
3 files changed, 56 insertions(+), 9 deletions(-)
diff --git a/contrib/pg_walinspect/expected/pg_walinspect.out b/contrib/pg_walinspect/expected/pg_walinspect.out
index b9abeeebdfd..e62af64014d 100644
--- a/contrib/pg_walinspect/expected/pg_walinspect.out
+++ b/contrib/pg_walinspect/expected/pg_walinspect.out
@@ -34,6 +34,33 @@ SELECT * FROM pg_get_wal_stats(:'wal_lsn2', :'wal_lsn1');
ERROR: WAL start LSN must be less than or equal to end LSN
SELECT * FROM pg_get_wal_block_info(:'wal_lsn2', :'wal_lsn1');
ERROR: WAL start LSN must be less than or equal to end LSN
+-- Empty ranges.
+SELECT pg_current_wal_flush_lsn() AS wal_lsn_end \gset
+SELECT COUNT(*) = 0 AS ok FROM pg_get_wal_records_info(:'wal_lsn_end', :'wal_lsn_end');
+ ok
+----
+ t
+(1 row)
+
+SELECT COUNT(*) >= 1 AND SUM(count) = 0 AS ok
+ FROM pg_get_wal_stats(:'wal_lsn_end', :'wal_lsn_end');
+ ok
+----
+ t
+(1 row)
+
+SELECT COUNT(*) = 0 AS ok FROM pg_get_wal_stats(:'wal_lsn_end', :'wal_lsn_end', true);
+ ok
+----
+ t
+(1 row)
+
+SELECT COUNT(*) = 0 AS ok FROM pg_get_wal_block_info(:'wal_lsn_end', :'wal_lsn_end');
+ ok
+----
+ t
+(1 row)
+
-- LSNs with the highest value possible.
SELECT * FROM pg_get_wal_record_info('FFFFFFFF/FFFFFFFF');
ERROR: WAL input LSN must be less than or equal to current LSN
diff --git a/contrib/pg_walinspect/pg_walinspect.c b/contrib/pg_walinspect/pg_walinspect.c
index 6edd6f8cb35..2e129bee9e2 100644
--- a/contrib/pg_walinspect/pg_walinspect.c
+++ b/contrib/pg_walinspect/pg_walinspect.c
@@ -435,6 +435,10 @@ pg_get_wal_block_info(PG_FUNCTION_ARGS)
InitMaterializedSRF(fcinfo, 0);
+ /* An empty range cannot contain any WAL records. */
+ if (start_lsn == end_lsn)
+ PG_RETURN_VOID();
+
xlogreader = InitXLogReaderState(start_lsn);
tmp_cxt = AllocSetContextCreate(CurrentMemoryContext,
@@ -560,6 +564,10 @@ GetWALRecordsInfo(FunctionCallInfo fcinfo, XLogRecPtr start_lsn,
InitMaterializedSRF(fcinfo, 0);
+ /* An empty range cannot contain any WAL records. */
+ if (start_lsn == end_lsn)
+ return;
+
xlogreader = InitXLogReaderState(start_lsn);
tmp_cxt = AllocSetContextCreate(CurrentMemoryContext,
@@ -787,18 +795,22 @@ GetWalStats(FunctionCallInfo fcinfo, XLogRecPtr start_lsn, XLogRecPtr end_lsn,
InitMaterializedSRF(fcinfo, 0);
- xlogreader = InitXLogReaderState(start_lsn);
-
- while (ReadNextXLogRecord(xlogreader) &&
- xlogreader->EndRecPtr <= end_lsn)
+ /* An empty range cannot contain any WAL records. */
+ if (start_lsn < end_lsn)
{
- XLogRecStoreStats(&stats, xlogreader);
+ xlogreader = InitXLogReaderState(start_lsn);
- CHECK_FOR_INTERRUPTS();
- }
+ while (ReadNextXLogRecord(xlogreader) &&
+ xlogreader->EndRecPtr <= end_lsn)
+ {
+ XLogRecStoreStats(&stats, xlogreader);
- pfree(xlogreader->private_data);
- XLogReaderFree(xlogreader);
+ CHECK_FOR_INTERRUPTS();
+ }
+
+ pfree(xlogreader->private_data);
+ XLogReaderFree(xlogreader);
+ }
GetXLogSummaryStats(&stats, rsinfo, values, nulls,
PG_GET_WAL_STATS_COLS,
diff --git a/contrib/pg_walinspect/sql/pg_walinspect.sql b/contrib/pg_walinspect/sql/pg_walinspect.sql
index 1e64a22d29a..903fdfe0554 100644
--- a/contrib/pg_walinspect/sql/pg_walinspect.sql
+++ b/contrib/pg_walinspect/sql/pg_walinspect.sql
@@ -31,6 +31,14 @@ SELECT * FROM pg_get_wal_records_info(:'wal_lsn2', :'wal_lsn1');
SELECT * FROM pg_get_wal_stats(:'wal_lsn2', :'wal_lsn1');
SELECT * FROM pg_get_wal_block_info(:'wal_lsn2', :'wal_lsn1');
+-- Empty ranges.
+SELECT pg_current_wal_flush_lsn() AS wal_lsn_end \gset
+SELECT COUNT(*) = 0 AS ok FROM pg_get_wal_records_info(:'wal_lsn_end', :'wal_lsn_end');
+SELECT COUNT(*) >= 1 AND SUM(count) = 0 AS ok
+ FROM pg_get_wal_stats(:'wal_lsn_end', :'wal_lsn_end');
+SELECT COUNT(*) = 0 AS ok FROM pg_get_wal_stats(:'wal_lsn_end', :'wal_lsn_end', true);
+SELECT COUNT(*) = 0 AS ok FROM pg_get_wal_block_info(:'wal_lsn_end', :'wal_lsn_end');
+
-- LSNs with the highest value possible.
SELECT * FROM pg_get_wal_record_info('FFFFFFFF/FFFFFFFF');
-- Success with end LSNs.
--
2.50.1 (Apple Git-155)
=
* Re: pg_walinspect: fix LSN validation messages and empty range handling
@ 2026-09-20 18:40 Kiran Kaki <itskkpg@gmail.com>
parent: Chao Li <li.evan.chao@gmail.com>
1 sibling, 1 reply; 16+ messages in thread
From: Kiran Kaki @ 2026-09-20 18:40 UTC (permalink / raw)
To: Chao Li <li.evan.chao@gmail.com>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Hi Chao
On Sat, Sep 19, 2026 at 8:08 PM Chao Li <li.evan.chao@gmail.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.
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."
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..
Thanks,
Kiran Kaki.
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: pg_walinspect: fix LSN validation messages and empty range handling
@ 2026-09-21 06:56 Chao Li <li.evan.chao@gmail.com>
parent: Kiran Kaki <itskkpg@gmail.com>
0 siblings, 2 replies; 16+ messages in thread
From: Chao Li @ 2026-09-21 06:56 UTC (permalink / raw)
To: Kiran Kaki <itskkpg@gmail.com>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
> On Sep 21, 2026, at 02:40, Kiran Kaki <itskkpg@gmail.com> wrote:
>
> Hi Chao
>
> On Sat, Sep 19, 2026 at 8:08 PM Chao Li <li.evan.chao@gmail.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/
Attachments:
v2-0001-Fix-inaccurate-LSN-validation-messages-in-pg_wali.patch (application/octet-stream, 6.1K) download
From d64d6f49e8935b4bde52510c02307c201fba8f6a Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <lic@highgo.com>
Date: Sun, 20 Sep 2026 09:39:56 +0800
Subject: [PATCH v2] Fix inaccurate LSN validation messages in pg_walinspect
Commit 5c1b6628075a changed the LSN validation checks to allow an input
LSN to equal the current LSN and a start LSN to equal the end LSN.
However, the corresponding error messages continued to say that the
LSNs had to be less than those bounds.
This was an oversight in 5c1b6628. Adjust the messages to say "less
than or equal to", matching the actual validation conditions.
Author: Chao Li <lic@highgo.com>
Reviewed-by: Kiran Kaki <itskkpg@gmail.com>
Discussion: https://postgr.es/m/7B8F5F12-98A5-4618-867A-904EA1334FD4@gmail.com
---
contrib/pg_walinspect/expected/oldextversions.out | 4 ++--
contrib/pg_walinspect/expected/pg_walinspect.out | 14 +++++++-------
contrib/pg_walinspect/pg_walinspect.c | 10 +++++-----
3 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/contrib/pg_walinspect/expected/oldextversions.out b/contrib/pg_walinspect/expected/oldextversions.out
index 89953af3d0e..fd65ed149c6 100644
--- a/contrib/pg_walinspect/expected/oldextversions.out
+++ b/contrib/pg_walinspect/expected/oldextversions.out
@@ -43,9 +43,9 @@ SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_stats_till_end_of_wal(:'wal_lsn1');
-- Failures with start LSNs.
SELECT * FROM pg_get_wal_records_info_till_end_of_wal('FFFFFFFF/FFFFFFFF');
-ERROR: WAL start LSN must be less than current LSN
+ERROR: WAL start LSN must be less than or equal to current LSN
SELECT * FROM pg_get_wal_stats_till_end_of_wal('FFFFFFFF/FFFFFFFF');
-ERROR: WAL start LSN must be less than current LSN
+ERROR: WAL start LSN must be less than or equal to current LSN
-- Move to new version 1.1.
ALTER EXTENSION pg_walinspect UPDATE TO '1.1';
-- List what version 1.1 contains.
diff --git a/contrib/pg_walinspect/expected/pg_walinspect.out b/contrib/pg_walinspect/expected/pg_walinspect.out
index f955ff5d3c5..b9abeeebdfd 100644
--- a/contrib/pg_walinspect/expected/pg_walinspect.out
+++ b/contrib/pg_walinspect/expected/pg_walinspect.out
@@ -29,14 +29,14 @@ SELECT * FROM pg_get_wal_block_info('0/0', :'wal_lsn1');
ERROR: could not read WAL at LSN 0/00000000
-- Start LSN > End LSN.
SELECT * FROM pg_get_wal_records_info(:'wal_lsn2', :'wal_lsn1');
-ERROR: WAL start LSN must be less than end LSN
+ERROR: WAL start LSN must be less than or equal to end LSN
SELECT * FROM pg_get_wal_stats(:'wal_lsn2', :'wal_lsn1');
-ERROR: WAL start LSN must be less than end LSN
+ERROR: WAL start LSN must be less than or equal to end LSN
SELECT * FROM pg_get_wal_block_info(:'wal_lsn2', :'wal_lsn1');
-ERROR: WAL start LSN must be less than end LSN
+ERROR: WAL start LSN must be less than or equal to end LSN
-- LSNs with the highest value possible.
SELECT * FROM pg_get_wal_record_info('FFFFFFFF/FFFFFFFF');
-ERROR: WAL input LSN must be less than current LSN
+ERROR: WAL input LSN must be less than or equal to current LSN
-- Success with end LSNs.
SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_records_info(:'wal_lsn1', 'FFFFFFFF/FFFFFFFF');
ok
@@ -58,11 +58,11 @@ SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_block_info(:'wal_lsn1', 'FFFFFFFF/FFF
-- Failures with start LSNs.
SELECT * FROM pg_get_wal_records_info('FFFFFFFF/FFFFFFFE', 'FFFFFFFF/FFFFFFFF');
-ERROR: WAL start LSN must be less than current LSN
+ERROR: WAL start LSN must be less than or equal to current LSN
SELECT * FROM pg_get_wal_stats('FFFFFFFF/FFFFFFFE', 'FFFFFFFF/FFFFFFFF');
-ERROR: WAL start LSN must be less than current LSN
+ERROR: WAL start LSN must be less than or equal to current LSN
SELECT * FROM pg_get_wal_block_info('FFFFFFFF/FFFFFFFE', 'FFFFFFFF/FFFFFFFF');
-ERROR: WAL start LSN must be less than current LSN
+ERROR: WAL start LSN must be less than or equal to current LSN
-- ===================================================================
-- Tests for all function executions
-- ===================================================================
diff --git a/contrib/pg_walinspect/pg_walinspect.c b/contrib/pg_walinspect/pg_walinspect.c
index a172f9e2b40..6edd6f8cb35 100644
--- a/contrib/pg_walinspect/pg_walinspect.c
+++ b/contrib/pg_walinspect/pg_walinspect.c
@@ -488,7 +488,7 @@ pg_get_wal_record_info(PG_FUNCTION_ARGS)
if (lsn > curr_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("WAL input LSN must be less than current LSN"),
+ errmsg("WAL input LSN must be less than or equal to current LSN"),
errdetail("Current WAL LSN on the database system is at %X/%08X.",
LSN_FORMAT_ARGS(curr_lsn))));
@@ -530,14 +530,14 @@ ValidateInputLSNs(XLogRecPtr start_lsn, XLogRecPtr *end_lsn)
if (start_lsn > curr_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("WAL start LSN must be less than current LSN"),
+ errmsg("WAL start LSN must be less than or equal to current LSN"),
errdetail("Current WAL LSN on the database system is at %X/%08X.",
LSN_FORMAT_ARGS(curr_lsn))));
if (start_lsn > *end_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("WAL start LSN must be less than end LSN")));
+ errmsg("WAL start LSN must be less than or equal to end LSN")));
if (*end_lsn > curr_lsn)
*end_lsn = curr_lsn;
@@ -836,7 +836,7 @@ pg_get_wal_records_info_till_end_of_wal(PG_FUNCTION_ARGS)
if (start_lsn > end_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("WAL start LSN must be less than current LSN"),
+ errmsg("WAL start LSN must be less than or equal to current LSN"),
errdetail("Current WAL LSN on the database system is at %X/%08X.",
LSN_FORMAT_ARGS(end_lsn))));
@@ -855,7 +855,7 @@ pg_get_wal_stats_till_end_of_wal(PG_FUNCTION_ARGS)
if (start_lsn > end_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("WAL start LSN must be less than current LSN"),
+ errmsg("WAL start LSN must be less than or equal to current LSN"),
errdetail("Current WAL LSN on the database system is at %X/%08X.",
LSN_FORMAT_ARGS(end_lsn))));
--
2.50.1 (Apple Git-155)
=
* Re: pg_walinspect: fix LSN validation messages and empty range handling
@ 2026-09-21 08:55 Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
parent: Chao Li <li.evan.chao@gmail.com>
1 sibling, 2 replies; 16+ messages in thread
From: Bharath Rupireddy @ 2026-09-21 08:55 UTC (permalink / raw)
To: Chao Li <li.evan.chao@gmail.com>; +Cc: Kiran Kaki <itskkpg@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Michael Paquier <michael@paquier.xyz>
Hi,
On Sun, Sep 20, 2026 at 11:56 PM Chao Li <li.evan.chao@gmail.com> wrote:
>
> PFA v2:
Thanks for reporting and sending the patch.
Yes, it's an oversight in 5c1b6628075a. +1 for "must be less than or
equal to", since that is the wording used elsewhere in the code.
That said, an error is raised only when no valid record is found at or
after the start LSN (or the input LSN), either because that WAL is
already removed or because nothing valid follows it, which is what the
documentation already mentions. Once a record is found,
pg_get_wal_record_info() emits it, whereas the range functions emit
only the records ending at or before the end LSN, so equal start and
end LSNs emit nothing. A start LSN equal to the current LSN ends up
the same way, since the end LSN is capped at the current LSN, making
the two equal, and it errors because nothing follows the current LSN.
The v2 patch looks good to me. I adjusted the commit message and
re-attached the patch, which I think is ready for commit. I'm fine not
back-patching this for a couple of reasons. The error is still
reported in the back-branches, just with slightly incorrect wording
matching the condition the code uses, and it went unnoticed for many
years. CC-ing Michael for any thoughts.
While here, do we also need to fix AlterSubscription()'s skip WAL
location and ParseVariableDouble()'s min and max bound messages? Maybe
separately.
[1]
DROP TBALE t1;
CREATE TABLE t1 (a int);
SELECT pg_current_wal_lsn() AS lsn \gset
INSERT INTO t1 VALUES (1);
-- start = end, the record there ends past end, no rows
SELECT * FROM pg_get_wal_records_info(:'lsn', :'lsn');
-- input at a valid record, one row
SELECT * FROM pg_get_wal_record_info(:'lsn');
-- start = end = current, nothing follows, error
SELECT * FROM pg_get_wal_records_info(pg_current_wal_lsn(),
pg_current_wal_lsn());
-- input = current, nothing follows, error
SELECT * FROM pg_get_wal_record_info(pg_current_wal_lsn());
-- start = current, end capped to current, so start = end again, error
SELECT * FROM pg_get_wal_records_info(pg_current_wal_lsn(),
'FFFFFFFF/FFFFFFFF');
-- recycle the segment holding :lsn
SELECT pg_switch_wal();
CHECKPOINT;
-- start = end, WAL already removed, error
SELECT * FROM pg_get_wal_records_info(:'lsn', :'lsn');
--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com
Attachments:
v2-0001-Fix-input-LSN-validation-error-messages-in-pg_wal.patch (application/octet-stream, 6.4K) download
From d64d6f49e8935b4bde52510c02307c201fba8f6a Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <lic@highgo.com>
Date: Sun, 20 Sep 2026 09:39:56 +0800
Subject: [PATCH v2] Fix input LSN validation error messages in pg_walinspect.
Commit 5c1b6628075a changed the input LSN validation checks to
allow an input LSN equal to the current LSN, and a start LSN equal
to the end LSN. However, the corresponding error messages
continued to say that the LSNs had to be less than the current LSN
or the end LSN.
Fix this oversight by changing those messages from "must be less
than" to "must be less than or equal to", matching the actual
validation conditions. Similar wording is used elsewhere in the
code.
Not back-patched, as the fix is not critical and the inaccuracy
went unnoticed for years.
Author: Chao Li <lic@highgo.com>
Reviewed-by: Kiran Kaki <itskkpg@gmail.com>
Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Discussion: https://postgr.es/m/7B8F5F12-98A5-4618-867A-904EA1334FD4@gmail.com
---
contrib/pg_walinspect/expected/oldextversions.out | 4 ++--
contrib/pg_walinspect/expected/pg_walinspect.out | 14 +++++++-------
contrib/pg_walinspect/pg_walinspect.c | 10 +++++-----
3 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/contrib/pg_walinspect/expected/oldextversions.out b/contrib/pg_walinspect/expected/oldextversions.out
index 89953af3d0e..fd65ed149c6 100644
--- a/contrib/pg_walinspect/expected/oldextversions.out
+++ b/contrib/pg_walinspect/expected/oldextversions.out
@@ -43,9 +43,9 @@ SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_stats_till_end_of_wal(:'wal_lsn1');
-- Failures with start LSNs.
SELECT * FROM pg_get_wal_records_info_till_end_of_wal('FFFFFFFF/FFFFFFFF');
-ERROR: WAL start LSN must be less than current LSN
+ERROR: WAL start LSN must be less than or equal to current LSN
SELECT * FROM pg_get_wal_stats_till_end_of_wal('FFFFFFFF/FFFFFFFF');
-ERROR: WAL start LSN must be less than current LSN
+ERROR: WAL start LSN must be less than or equal to current LSN
-- Move to new version 1.1.
ALTER EXTENSION pg_walinspect UPDATE TO '1.1';
-- List what version 1.1 contains.
diff --git a/contrib/pg_walinspect/expected/pg_walinspect.out b/contrib/pg_walinspect/expected/pg_walinspect.out
index f955ff5d3c5..b9abeeebdfd 100644
--- a/contrib/pg_walinspect/expected/pg_walinspect.out
+++ b/contrib/pg_walinspect/expected/pg_walinspect.out
@@ -29,14 +29,14 @@ SELECT * FROM pg_get_wal_block_info('0/0', :'wal_lsn1');
ERROR: could not read WAL at LSN 0/00000000
-- Start LSN > End LSN.
SELECT * FROM pg_get_wal_records_info(:'wal_lsn2', :'wal_lsn1');
-ERROR: WAL start LSN must be less than end LSN
+ERROR: WAL start LSN must be less than or equal to end LSN
SELECT * FROM pg_get_wal_stats(:'wal_lsn2', :'wal_lsn1');
-ERROR: WAL start LSN must be less than end LSN
+ERROR: WAL start LSN must be less than or equal to end LSN
SELECT * FROM pg_get_wal_block_info(:'wal_lsn2', :'wal_lsn1');
-ERROR: WAL start LSN must be less than end LSN
+ERROR: WAL start LSN must be less than or equal to end LSN
-- LSNs with the highest value possible.
SELECT * FROM pg_get_wal_record_info('FFFFFFFF/FFFFFFFF');
-ERROR: WAL input LSN must be less than current LSN
+ERROR: WAL input LSN must be less than or equal to current LSN
-- Success with end LSNs.
SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_records_info(:'wal_lsn1', 'FFFFFFFF/FFFFFFFF');
ok
@@ -58,11 +58,11 @@ SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_block_info(:'wal_lsn1', 'FFFFFFFF/FFF
-- Failures with start LSNs.
SELECT * FROM pg_get_wal_records_info('FFFFFFFF/FFFFFFFE', 'FFFFFFFF/FFFFFFFF');
-ERROR: WAL start LSN must be less than current LSN
+ERROR: WAL start LSN must be less than or equal to current LSN
SELECT * FROM pg_get_wal_stats('FFFFFFFF/FFFFFFFE', 'FFFFFFFF/FFFFFFFF');
-ERROR: WAL start LSN must be less than current LSN
+ERROR: WAL start LSN must be less than or equal to current LSN
SELECT * FROM pg_get_wal_block_info('FFFFFFFF/FFFFFFFE', 'FFFFFFFF/FFFFFFFF');
-ERROR: WAL start LSN must be less than current LSN
+ERROR: WAL start LSN must be less than or equal to current LSN
-- ===================================================================
-- Tests for all function executions
-- ===================================================================
diff --git a/contrib/pg_walinspect/pg_walinspect.c b/contrib/pg_walinspect/pg_walinspect.c
index a172f9e2b40..6edd6f8cb35 100644
--- a/contrib/pg_walinspect/pg_walinspect.c
+++ b/contrib/pg_walinspect/pg_walinspect.c
@@ -488,7 +488,7 @@ pg_get_wal_record_info(PG_FUNCTION_ARGS)
if (lsn > curr_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("WAL input LSN must be less than current LSN"),
+ errmsg("WAL input LSN must be less than or equal to current LSN"),
errdetail("Current WAL LSN on the database system is at %X/%08X.",
LSN_FORMAT_ARGS(curr_lsn))));
@@ -530,14 +530,14 @@ ValidateInputLSNs(XLogRecPtr start_lsn, XLogRecPtr *end_lsn)
if (start_lsn > curr_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("WAL start LSN must be less than current LSN"),
+ errmsg("WAL start LSN must be less than or equal to current LSN"),
errdetail("Current WAL LSN on the database system is at %X/%08X.",
LSN_FORMAT_ARGS(curr_lsn))));
if (start_lsn > *end_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("WAL start LSN must be less than end LSN")));
+ errmsg("WAL start LSN must be less than or equal to end LSN")));
if (*end_lsn > curr_lsn)
*end_lsn = curr_lsn;
@@ -836,7 +836,7 @@ pg_get_wal_records_info_till_end_of_wal(PG_FUNCTION_ARGS)
if (start_lsn > end_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("WAL start LSN must be less than current LSN"),
+ errmsg("WAL start LSN must be less than or equal to current LSN"),
errdetail("Current WAL LSN on the database system is at %X/%08X.",
LSN_FORMAT_ARGS(end_lsn))));
@@ -855,7 +855,7 @@ pg_get_wal_stats_till_end_of_wal(PG_FUNCTION_ARGS)
if (start_lsn > end_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("WAL start LSN must be less than current LSN"),
+ errmsg("WAL start LSN must be less than or equal to current LSN"),
errdetail("Current WAL LSN on the database system is at %X/%08X.",
LSN_FORMAT_ARGS(end_lsn))));
--
2.47.3
* Re: pg_walinspect: fix LSN validation messages and empty range handling
@ 2026-09-21 15:28 Kiran Kaki <itskkpg@gmail.com>
parent: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
1 sibling, 0 replies; 16+ messages in thread
From: Kiran Kaki @ 2026-09-21 15:28 UTC (permalink / raw)
To: bharath.rupireddyforpostgres@gmail.com; +Cc: li.evan.chao@gmail.com; pgsql-hackers@lists.postgresql.org; michael@paquier.xyz
👍
Kiran reacted via Gmail
<https://www.google.com/gmail/about/?utm_source=gmail-in-product&utm_medium=et&utm_campaign=e...;
On Mon, Sep 21, 2026, 1:55 AM Bharath Rupireddy <
bharath.rupireddyforpostgres@gmail.com> wrote:
> Hi,
>
> On Sun, Sep 20, 2026 at 11:56 PM Chao Li <li.evan.chao@gmail.com> wrote:
> >
> > PFA v2:
>
> Thanks for reporting and sending the patch.
>
> Yes, it's an oversight in 5c1b6628075a. +1 for "must be less than or
> equal to", since that is the wording used elsewhere in the code.
>
> That said, an error is raised only when no valid record is found at or
> after the start LSN (or the input LSN), either because that WAL is
> already removed or because nothing valid follows it, which is what the
> documentation already mentions. Once a record is found,
> pg_get_wal_record_info() emits it, whereas the range functions emit
> only the records ending at or before the end LSN, so equal start and
> end LSNs emit nothing. A start LSN equal to the current LSN ends up
> the same way, since the end LSN is capped at the current LSN, making
> the two equal, and it errors because nothing follows the current LSN.
>
> The v2 patch looks good to me. I adjusted the commit message and
> re-attached the patch, which I think is ready for commit. I'm fine not
> back-patching this for a couple of reasons. The error is still
> reported in the back-branches, just with slightly incorrect wording
> matching the condition the code uses, and it went unnoticed for many
> years. CC-ing Michael for any thoughts.
>
> While here, do we also need to fix AlterSubscription()'s skip WAL
> location and ParseVariableDouble()'s min and max bound messages? Maybe
> separately.
>
> [1]
> DROP TBALE t1;
> CREATE TABLE t1 (a int);
> SELECT pg_current_wal_lsn() AS lsn \gset
> INSERT INTO t1 VALUES (1);
>
> -- start = end, the record there ends past end, no rows
> SELECT * FROM pg_get_wal_records_info(:'lsn', :'lsn');
>
> -- input at a valid record, one row
> SELECT * FROM pg_get_wal_record_info(:'lsn');
>
> -- start = end = current, nothing follows, error
> SELECT * FROM pg_get_wal_records_info(pg_current_wal_lsn(),
> pg_current_wal_lsn());
>
> -- input = current, nothing follows, error
> SELECT * FROM pg_get_wal_record_info(pg_current_wal_lsn());
>
> -- start = current, end capped to current, so start = end again, error
> SELECT * FROM pg_get_wal_records_info(pg_current_wal_lsn(),
> 'FFFFFFFF/FFFFFFFF');
>
> -- recycle the segment holding :lsn
> SELECT pg_switch_wal();
> CHECKPOINT;
>
> -- start = end, WAL already removed, error
> SELECT * FROM pg_get_wal_records_info(:'lsn', :'lsn');
>
> --
> Bharath Rupireddy
> Amazon Web Services: https://aws.amazon.com
>
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: pg_walinspect: fix LSN validation messages and empty range handling
@ 2026-09-21 15:29 Kiran Kaki <itskkpg@gmail.com>
parent: Chao Li <li.evan.chao@gmail.com>
1 sibling, 0 replies; 16+ messages in thread
From: Kiran Kaki @ 2026-09-21 15:29 UTC (permalink / raw)
To: li.evan.chao@gmail.com; +Cc: pgsql-hackers@lists.postgresql.org; bharath.rupireddyforpostgres@gmail.com
👍
Kiran reacted via Gmail
<https://www.google.com/gmail/about/?utm_source=gmail-in-product&utm_medium=et&utm_campaign=e...;
On Sun, Sep 20, 2026, 11:56 PM Chao Li <li.evan.chao@gmail.com> wrote:
>
>
> > On Sep 21, 2026, at 02:40, Kiran Kaki <itskkpg@gmail.com> wrote:
> >
> > Hi Chao
> >
> > On Sat, Sep 19, 2026 at 8:08 PM Chao Li <li.evan.chao@gmail.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/
>
>
>
>
>
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: pg_walinspect: fix LSN validation messages and empty range handling
@ 2026-09-21 22:50 Chao Li <li.evan.chao@gmail.com>
parent: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
1 sibling, 1 reply; 16+ messages in thread
From: Chao Li @ 2026-09-21 22:50 UTC (permalink / raw)
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>; +Cc: Kiran Kaki <itskkpg@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Michael Paquier <michael@paquier.xyz>
> On Sep 21, 2026, at 16:55, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote:
>
> Hi,
>
> On Sun, Sep 20, 2026 at 11:56 PM Chao Li <li.evan.chao@gmail.com> wrote:
>>
>> PFA v2:
>
> Thanks for reporting and sending the patch.
>
> Yes, it's an oversight in 5c1b6628075a. +1 for "must be less than or
> equal to", since that is the wording used elsewhere in the code.
>
> That said, an error is raised only when no valid record is found at or
> after the start LSN (or the input LSN), either because that WAL is
> already removed or because nothing valid follows it, which is what the
> documentation already mentions. Once a record is found,
> pg_get_wal_record_info() emits it, whereas the range functions emit
> only the records ending at or before the end LSN, so equal start and
> end LSNs emit nothing. A start LSN equal to the current LSN ends up
> the same way, since the end LSN is capped at the current LSN, making
> the two equal, and it errors because nothing follows the current LSN.
>
> The v2 patch looks good to me. I adjusted the commit message and
> re-attached the patch, which I think is ready for commit. I'm fine not
> back-patching this for a couple of reasons. The error is still
> reported in the back-branches, just with slightly incorrect wording
> matching the condition the code uses, and it went unnoticed for many
> years. CC-ing Michael for any thoughts.
Thanks for reviewing and updating v2.
>
> While here, do we also need to fix AlterSubscription()'s skip WAL
> location and ParseVariableDouble()'s min and max bound messages? Maybe
> separately.
>
Yeah, we can do that. As those two functions are in core, and 0001 changes the extension, I put the new changes to 0002.
PFA v3.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Attachments: download all (2 files, .tar.gz)
v3-0001-Fix-input-LSN-validation-error-messages-in-pg_wal.patch (application/octet-stream, 6.4K) download
From 45270a5e8f24292286b81e39fe2e9f401d04ce43 Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <lic@highgo.com>
Date: Sun, 20 Sep 2026 09:39:56 +0800
Subject: [PATCH v3 1/2] Fix input LSN validation error messages in
pg_walinspect.
Commit 5c1b6628075a changed the input LSN validation checks to allow an input
LSN equal to the current LSN, and a start LSN equal to the end LSN. However,
the corresponding error messages continued to say that the LSNs had to be
less than the current LSN or the end LSN.
Fix this oversight by changing those messages from "must be less than" to
"must be less than or equal to", matching the actual validation conditions.
Similar wording is used elsewhere in the code.
Not back-patched, as the fix is not critical and the inaccuracy went unnoticed
for years.
Author: Chao Li <lic@highgo.com>
Reviewed-by: Kiran Kaki <itskkpg@gmail.com>
Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Discussion: https://postgr.es/m/7B8F5F12-98A5-4618-867A-904EA1334FD4@gmail.com
---
contrib/pg_walinspect/expected/oldextversions.out | 4 ++--
contrib/pg_walinspect/expected/pg_walinspect.out | 14 +++++++-------
contrib/pg_walinspect/pg_walinspect.c | 10 +++++-----
3 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/contrib/pg_walinspect/expected/oldextversions.out b/contrib/pg_walinspect/expected/oldextversions.out
index 89953af3d0e..fd65ed149c6 100644
--- a/contrib/pg_walinspect/expected/oldextversions.out
+++ b/contrib/pg_walinspect/expected/oldextversions.out
@@ -43,9 +43,9 @@ SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_stats_till_end_of_wal(:'wal_lsn1');
-- Failures with start LSNs.
SELECT * FROM pg_get_wal_records_info_till_end_of_wal('FFFFFFFF/FFFFFFFF');
-ERROR: WAL start LSN must be less than current LSN
+ERROR: WAL start LSN must be less than or equal to current LSN
SELECT * FROM pg_get_wal_stats_till_end_of_wal('FFFFFFFF/FFFFFFFF');
-ERROR: WAL start LSN must be less than current LSN
+ERROR: WAL start LSN must be less than or equal to current LSN
-- Move to new version 1.1.
ALTER EXTENSION pg_walinspect UPDATE TO '1.1';
-- List what version 1.1 contains.
diff --git a/contrib/pg_walinspect/expected/pg_walinspect.out b/contrib/pg_walinspect/expected/pg_walinspect.out
index f955ff5d3c5..b9abeeebdfd 100644
--- a/contrib/pg_walinspect/expected/pg_walinspect.out
+++ b/contrib/pg_walinspect/expected/pg_walinspect.out
@@ -29,14 +29,14 @@ SELECT * FROM pg_get_wal_block_info('0/0', :'wal_lsn1');
ERROR: could not read WAL at LSN 0/00000000
-- Start LSN > End LSN.
SELECT * FROM pg_get_wal_records_info(:'wal_lsn2', :'wal_lsn1');
-ERROR: WAL start LSN must be less than end LSN
+ERROR: WAL start LSN must be less than or equal to end LSN
SELECT * FROM pg_get_wal_stats(:'wal_lsn2', :'wal_lsn1');
-ERROR: WAL start LSN must be less than end LSN
+ERROR: WAL start LSN must be less than or equal to end LSN
SELECT * FROM pg_get_wal_block_info(:'wal_lsn2', :'wal_lsn1');
-ERROR: WAL start LSN must be less than end LSN
+ERROR: WAL start LSN must be less than or equal to end LSN
-- LSNs with the highest value possible.
SELECT * FROM pg_get_wal_record_info('FFFFFFFF/FFFFFFFF');
-ERROR: WAL input LSN must be less than current LSN
+ERROR: WAL input LSN must be less than or equal to current LSN
-- Success with end LSNs.
SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_records_info(:'wal_lsn1', 'FFFFFFFF/FFFFFFFF');
ok
@@ -58,11 +58,11 @@ SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_block_info(:'wal_lsn1', 'FFFFFFFF/FFF
-- Failures with start LSNs.
SELECT * FROM pg_get_wal_records_info('FFFFFFFF/FFFFFFFE', 'FFFFFFFF/FFFFFFFF');
-ERROR: WAL start LSN must be less than current LSN
+ERROR: WAL start LSN must be less than or equal to current LSN
SELECT * FROM pg_get_wal_stats('FFFFFFFF/FFFFFFFE', 'FFFFFFFF/FFFFFFFF');
-ERROR: WAL start LSN must be less than current LSN
+ERROR: WAL start LSN must be less than or equal to current LSN
SELECT * FROM pg_get_wal_block_info('FFFFFFFF/FFFFFFFE', 'FFFFFFFF/FFFFFFFF');
-ERROR: WAL start LSN must be less than current LSN
+ERROR: WAL start LSN must be less than or equal to current LSN
-- ===================================================================
-- Tests for all function executions
-- ===================================================================
diff --git a/contrib/pg_walinspect/pg_walinspect.c b/contrib/pg_walinspect/pg_walinspect.c
index a172f9e2b40..6edd6f8cb35 100644
--- a/contrib/pg_walinspect/pg_walinspect.c
+++ b/contrib/pg_walinspect/pg_walinspect.c
@@ -488,7 +488,7 @@ pg_get_wal_record_info(PG_FUNCTION_ARGS)
if (lsn > curr_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("WAL input LSN must be less than current LSN"),
+ errmsg("WAL input LSN must be less than or equal to current LSN"),
errdetail("Current WAL LSN on the database system is at %X/%08X.",
LSN_FORMAT_ARGS(curr_lsn))));
@@ -530,14 +530,14 @@ ValidateInputLSNs(XLogRecPtr start_lsn, XLogRecPtr *end_lsn)
if (start_lsn > curr_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("WAL start LSN must be less than current LSN"),
+ errmsg("WAL start LSN must be less than or equal to current LSN"),
errdetail("Current WAL LSN on the database system is at %X/%08X.",
LSN_FORMAT_ARGS(curr_lsn))));
if (start_lsn > *end_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("WAL start LSN must be less than end LSN")));
+ errmsg("WAL start LSN must be less than or equal to end LSN")));
if (*end_lsn > curr_lsn)
*end_lsn = curr_lsn;
@@ -836,7 +836,7 @@ pg_get_wal_records_info_till_end_of_wal(PG_FUNCTION_ARGS)
if (start_lsn > end_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("WAL start LSN must be less than current LSN"),
+ errmsg("WAL start LSN must be less than or equal to current LSN"),
errdetail("Current WAL LSN on the database system is at %X/%08X.",
LSN_FORMAT_ARGS(end_lsn))));
@@ -855,7 +855,7 @@ pg_get_wal_stats_till_end_of_wal(PG_FUNCTION_ARGS)
if (start_lsn > end_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("WAL start LSN must be less than current LSN"),
+ errmsg("WAL start LSN must be less than or equal to current LSN"),
errdetail("Current WAL LSN on the database system is at %X/%08X.",
LSN_FORMAT_ARGS(end_lsn))));
--
2.50.1 (Apple Git-155)
=
v3-0002-Fix-inaccurate-inclusive-bound-error-messages.patch (application/octet-stream, 5.1K) download
From 44ab84440218b2d1d86af088a340e1ee19bdcba1 Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <lic@highgo.com>
Date: Mon, 21 Sep 2026 17:31:14 +0800
Subject: [PATCH v3 2/2] Fix inaccurate inclusive-bound error messages.
ALTER SUBSCRIPTION ... SKIP accepts a skip LSN equal to the replication
origin's progress, but its error message said that the skip LSN must be
greater than the origin LSN. Likewise, ParseVariableDouble() accepts
values at either end of its documented [min,max] range, but its
messages said that they had to be strictly inside the range.
Adjust the messages to describe the inclusive bounds accurately, and
add coverage for both cases.
Suggested-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Author: Chao Li <lic@highgo.com>
Discussion: https://postgr.es/m/7B8F5F12-98A5-4618-867A-904EA1334FD4@gmail.com
---
src/backend/commands/subscriptioncmds.c | 2 +-
src/bin/psql/t/001_basic.pl | 10 ++++++++++
src/bin/psql/variables.c | 4 ++--
src/test/regress/expected/subscription.out | 8 ++++++++
src/test/regress/sql/subscription.sql | 8 ++++++++
5 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 22a61dca65d..27a5b55e964 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -2419,7 +2419,7 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt,
if (XLogRecPtrIsValid(remote_lsn) && opts.lsn < remote_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("skip WAL location (LSN %X/%08X) must be greater than origin LSN %X/%08X",
+ errmsg("skip WAL location (LSN %X/%08X) must be greater than or equal to origin LSN %X/%08X",
LSN_FORMAT_ARGS(opts.lsn),
LSN_FORMAT_ARGS(remote_lsn))));
}
diff --git a/src/bin/psql/t/001_basic.pl b/src/bin/psql/t/001_basic.pl
index 028df33ce8a..0661dd6d85f 100644
--- a/src/bin/psql/t/001_basic.pl
+++ b/src/bin/psql/t/001_basic.pl
@@ -452,6 +452,16 @@ psql_fails_like(
'\set WATCH_INTERVAL 1e500',
qr/is out of range/,
'WATCH_INTERVAL variable is out of range');
+psql_fails_like(
+ $node,
+ '\set WATCH_INTERVAL -1',
+ qr/must be greater than or equal to 0\.00/,
+ 'WATCH_INTERVAL variable is below its lower bound');
+psql_fails_like(
+ $node,
+ '\set WATCH_INTERVAL 1000001',
+ qr/must be less than or equal to 1000000\.00/,
+ 'WATCH_INTERVAL variable is above its upper bound');
psql_like($node, '\echo :WATCH_INTERVAL',
qr/^2$/m, 'WATCH_INTERVAL variable was not altered');
diff --git a/src/bin/psql/variables.c b/src/bin/psql/variables.c
index 8060f2959cc..2c9a191a69b 100644
--- a/src/bin/psql/variables.c
+++ b/src/bin/psql/variables.c
@@ -215,14 +215,14 @@ ParseVariableDouble(const char *value, const char *name, double *result, double
if (dblval < min)
{
if (name)
- pg_log_error("invalid value \"%s\" for variable \"%s\": must be greater than %.2f",
+ pg_log_error("invalid value \"%s\" for variable \"%s\": must be greater than or equal to %.2f",
value, name, min);
return false;
}
else if (dblval > max)
{
if (name)
- pg_log_error("invalid value \"%s\" for variable \"%s\": must be less than %.2f",
+ pg_log_error("invalid value \"%s\" for variable \"%s\": must be less than or equal to %.2f",
value, name, max);
return false;
}
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index 681dc66dca6..2ccb4e550d3 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -294,6 +294,14 @@ ALTER SUBSCRIPTION regress_testsub SKIP (lsn = '0/12345');
-- ok - with lsn = NONE
ALTER SUBSCRIPTION regress_testsub SKIP (lsn = NONE);
+-- fail - LSN must not be behind the replication origin
+RESET SESSION AUTHORIZATION;
+SELECT pg_replication_origin_advance(
+ 'pg_' || (SELECT oid FROM pg_subscription WHERE subname = 'regress_testsub'),
+ '0/12346') \gset
+ALTER SUBSCRIPTION regress_testsub SKIP (lsn = '0/12345');
+ERROR: skip WAL location (LSN 0/00012345) must be greater than or equal to origin LSN 0/00012346
+SET SESSION AUTHORIZATION regress_subscription_user;
-- fail
ALTER SUBSCRIPTION regress_testsub SKIP (lsn = '0/0');
ERROR: invalid WAL location (LSN): 0/0
diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql
index cfeebaf9302..029ece91ea5 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -231,6 +231,14 @@ ALTER SUBSCRIPTION regress_testsub SKIP (lsn = '0/12345');
-- ok - with lsn = NONE
ALTER SUBSCRIPTION regress_testsub SKIP (lsn = NONE);
+-- fail - LSN must not be behind the replication origin
+RESET SESSION AUTHORIZATION;
+SELECT pg_replication_origin_advance(
+ 'pg_' || (SELECT oid FROM pg_subscription WHERE subname = 'regress_testsub'),
+ '0/12346') \gset
+ALTER SUBSCRIPTION regress_testsub SKIP (lsn = '0/12345');
+SET SESSION AUTHORIZATION regress_subscription_user;
+
-- fail
ALTER SUBSCRIPTION regress_testsub SKIP (lsn = '0/0');
--
2.50.1 (Apple Git-155)
=
* Re: pg_walinspect: fix LSN validation messages and empty range handling
@ 2026-09-22 02:06 Kiran Kaki <itskkpg@gmail.com>
parent: Chao Li <li.evan.chao@gmail.com>
0 siblings, 1 reply; 16+ messages in thread
From: Kiran Kaki @ 2026-09-22 02:06 UTC (permalink / raw)
To: Chao Li <li.evan.chao@gmail.com>; +Cc: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Michael Paquier <michael@paquier.xyz>
Hi @Chao Li
On Mon, Sep 21, 2026 at 3:51 PM Chao Li <li.evan.chao@gmail.com> wrote:
> Thanks for reviewing and updating v2.
>
> >
> > While here, do we also need to fix AlterSubscription()'s skip WAL
> > location and ParseVariableDouble()'s min and max bound messages? Maybe
> > separately.
> >
>
> Yeah, we can do that. As those two functions are in core, and 0001 changes the extension, I put the new changes to 0002.
>
> PFA v3.
Tested both patches on master (d39fda1cc4). They apply cleanly, build
without warnings, and the full suite passes (363 ok, 0 failed). I also
confirmed each reworded message at runtime, including the inclusive
boundaries they describe.
0001 is unchanged since v1, only the commit message differs. Looks good to me.
For 0002, the comment above that check already says "at least a future
LSN" and came in with the code in 208c5d65bb, so the reworded message
matches what the check was always meant to do. Looks good to me.
One suggestion, not a blocker. The new tests only cover the cases that
fail. The lower bound is already covered by the existing "Test zero
interval" case in 001_basic.pl, but the upper bound and the equal LSN
case are not, so they would still pass if those bounds were made
exclusive. Two additions would close that:
psql_like(
$node,
'\set WATCH_INTERVAL 1000000' . "\n" . '\echo :WATCH_INTERVAL',
qr/^1000000$/m,
'WATCH_INTERVAL accepts its upper bound');
-- ok - LSN equal to the replication origin is accepted
ALTER SUBSCRIPTION regress_testsub SKIP (lsn = '0/12346');
ALTER SUBSCRIPTION regress_testsub SKIP (lsn = NONE);
The second line just resets the value so the rest of the file's expected
output is unaffected.
Both patches are correct as they stand, so this is ready for a committer
either way. The test additions are optional, so please see if you want
to take them before committing.
Thanks Bharath for the review comments upthread, they made the scope of
these changes much easier to follow.
Regards,
Kiran Kaki
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: pg_walinspect: fix LSN validation messages and empty range handling
@ 2026-09-22 05:20 Chao Li <li.evan.chao@gmail.com>
parent: Kiran Kaki <itskkpg@gmail.com>
0 siblings, 0 replies; 16+ messages in thread
From: Chao Li @ 2026-09-22 05:20 UTC (permalink / raw)
To: Kiran Kaki <itskkpg@gmail.com>; +Cc: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Michael Paquier <michael@paquier.xyz>
> On Sep 22, 2026, at 10:06, Kiran Kaki <itskkpg@gmail.com> wrote:
>
> Hi @Chao Li
>
> On Mon, Sep 21, 2026 at 3:51 PM Chao Li <li.evan.chao@gmail.com> wrote:
>> Thanks for reviewing and updating v2.
>>
>>>
>>> While here, do we also need to fix AlterSubscription()'s skip WAL
>>> location and ParseVariableDouble()'s min and max bound messages? Maybe
>>> separately.
>>>
>>
>> Yeah, we can do that. As those two functions are in core, and 0001 changes the extension, I put the new changes to 0002.
>>
>> PFA v3.
>
> Tested both patches on master (d39fda1cc4). They apply cleanly, build
> without warnings, and the full suite passes (363 ok, 0 failed). I also
> confirmed each reworded message at runtime, including the inclusive
> boundaries they describe.
>
> 0001 is unchanged since v1, only the commit message differs. Looks good to me.
>
> For 0002, the comment above that check already says "at least a future
> LSN" and came in with the code in 208c5d65bb, so the reworded message
> matches what the check was always meant to do. Looks good to me.
>
> One suggestion, not a blocker. The new tests only cover the cases that
> fail. The lower bound is already covered by the existing "Test zero
> interval" case in 001_basic.pl, but the upper bound and the equal LSN
> case are not, so they would still pass if those bounds were made
> exclusive. Two additions would close that:
>
> psql_like(
> $node,
> '\set WATCH_INTERVAL 1000000' . "\n" . '\echo :WATCH_INTERVAL',
> qr/^1000000$/m,
> 'WATCH_INTERVAL accepts its upper bound');
>
> -- ok - LSN equal to the replication origin is accepted
> ALTER SUBSCRIPTION regress_testsub SKIP (lsn = '0/12346');
> ALTER SUBSCRIPTION regress_testsub SKIP (lsn = NONE);
>
> The second line just resets the value so the rest of the file's expected
> output is unaffected.
Integrated the tests into v4.
PFA v4:
* 0001 unchanged
* 0002 added the tests that Kiran suggested
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Attachments: download all (2 files, .tar.gz)
v4-0001-Fix-input-LSN-validation-error-messages-in-pg_wal.patch (application/octet-stream, 6.4K) download
From 190fe2e1887cc480ea496bbe09ebb1ece34631b3 Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <lic@highgo.com>
Date: Sun, 20 Sep 2026 09:39:56 +0800
Subject: [PATCH v4 1/2] Fix input LSN validation error messages in
pg_walinspect.
Commit 5c1b6628075a changed the input LSN validation checks to allow an input
LSN equal to the current LSN, and a start LSN equal to the end LSN. However,
the corresponding error messages continued to say that the LSNs had to be
less than the current LSN or the end LSN.
Fix this oversight by changing those messages from "must be less than" to
"must be less than or equal to", matching the actual validation conditions.
Similar wording is used elsewhere in the code.
Not back-patched, as the fix is not critical and the inaccuracy went unnoticed
for years.
Author: Chao Li <lic@highgo.com>
Reviewed-by: Kiran Kaki <itskkpg@gmail.com>
Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Discussion: https://postgr.es/m/7B8F5F12-98A5-4618-867A-904EA1334FD4@gmail.com
---
contrib/pg_walinspect/expected/oldextversions.out | 4 ++--
contrib/pg_walinspect/expected/pg_walinspect.out | 14 +++++++-------
contrib/pg_walinspect/pg_walinspect.c | 10 +++++-----
3 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/contrib/pg_walinspect/expected/oldextversions.out b/contrib/pg_walinspect/expected/oldextversions.out
index 89953af3d0e..fd65ed149c6 100644
--- a/contrib/pg_walinspect/expected/oldextversions.out
+++ b/contrib/pg_walinspect/expected/oldextversions.out
@@ -43,9 +43,9 @@ SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_stats_till_end_of_wal(:'wal_lsn1');
-- Failures with start LSNs.
SELECT * FROM pg_get_wal_records_info_till_end_of_wal('FFFFFFFF/FFFFFFFF');
-ERROR: WAL start LSN must be less than current LSN
+ERROR: WAL start LSN must be less than or equal to current LSN
SELECT * FROM pg_get_wal_stats_till_end_of_wal('FFFFFFFF/FFFFFFFF');
-ERROR: WAL start LSN must be less than current LSN
+ERROR: WAL start LSN must be less than or equal to current LSN
-- Move to new version 1.1.
ALTER EXTENSION pg_walinspect UPDATE TO '1.1';
-- List what version 1.1 contains.
diff --git a/contrib/pg_walinspect/expected/pg_walinspect.out b/contrib/pg_walinspect/expected/pg_walinspect.out
index f955ff5d3c5..b9abeeebdfd 100644
--- a/contrib/pg_walinspect/expected/pg_walinspect.out
+++ b/contrib/pg_walinspect/expected/pg_walinspect.out
@@ -29,14 +29,14 @@ SELECT * FROM pg_get_wal_block_info('0/0', :'wal_lsn1');
ERROR: could not read WAL at LSN 0/00000000
-- Start LSN > End LSN.
SELECT * FROM pg_get_wal_records_info(:'wal_lsn2', :'wal_lsn1');
-ERROR: WAL start LSN must be less than end LSN
+ERROR: WAL start LSN must be less than or equal to end LSN
SELECT * FROM pg_get_wal_stats(:'wal_lsn2', :'wal_lsn1');
-ERROR: WAL start LSN must be less than end LSN
+ERROR: WAL start LSN must be less than or equal to end LSN
SELECT * FROM pg_get_wal_block_info(:'wal_lsn2', :'wal_lsn1');
-ERROR: WAL start LSN must be less than end LSN
+ERROR: WAL start LSN must be less than or equal to end LSN
-- LSNs with the highest value possible.
SELECT * FROM pg_get_wal_record_info('FFFFFFFF/FFFFFFFF');
-ERROR: WAL input LSN must be less than current LSN
+ERROR: WAL input LSN must be less than or equal to current LSN
-- Success with end LSNs.
SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_records_info(:'wal_lsn1', 'FFFFFFFF/FFFFFFFF');
ok
@@ -58,11 +58,11 @@ SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_block_info(:'wal_lsn1', 'FFFFFFFF/FFF
-- Failures with start LSNs.
SELECT * FROM pg_get_wal_records_info('FFFFFFFF/FFFFFFFE', 'FFFFFFFF/FFFFFFFF');
-ERROR: WAL start LSN must be less than current LSN
+ERROR: WAL start LSN must be less than or equal to current LSN
SELECT * FROM pg_get_wal_stats('FFFFFFFF/FFFFFFFE', 'FFFFFFFF/FFFFFFFF');
-ERROR: WAL start LSN must be less than current LSN
+ERROR: WAL start LSN must be less than or equal to current LSN
SELECT * FROM pg_get_wal_block_info('FFFFFFFF/FFFFFFFE', 'FFFFFFFF/FFFFFFFF');
-ERROR: WAL start LSN must be less than current LSN
+ERROR: WAL start LSN must be less than or equal to current LSN
-- ===================================================================
-- Tests for all function executions
-- ===================================================================
diff --git a/contrib/pg_walinspect/pg_walinspect.c b/contrib/pg_walinspect/pg_walinspect.c
index a172f9e2b40..6edd6f8cb35 100644
--- a/contrib/pg_walinspect/pg_walinspect.c
+++ b/contrib/pg_walinspect/pg_walinspect.c
@@ -488,7 +488,7 @@ pg_get_wal_record_info(PG_FUNCTION_ARGS)
if (lsn > curr_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("WAL input LSN must be less than current LSN"),
+ errmsg("WAL input LSN must be less than or equal to current LSN"),
errdetail("Current WAL LSN on the database system is at %X/%08X.",
LSN_FORMAT_ARGS(curr_lsn))));
@@ -530,14 +530,14 @@ ValidateInputLSNs(XLogRecPtr start_lsn, XLogRecPtr *end_lsn)
if (start_lsn > curr_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("WAL start LSN must be less than current LSN"),
+ errmsg("WAL start LSN must be less than or equal to current LSN"),
errdetail("Current WAL LSN on the database system is at %X/%08X.",
LSN_FORMAT_ARGS(curr_lsn))));
if (start_lsn > *end_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("WAL start LSN must be less than end LSN")));
+ errmsg("WAL start LSN must be less than or equal to end LSN")));
if (*end_lsn > curr_lsn)
*end_lsn = curr_lsn;
@@ -836,7 +836,7 @@ pg_get_wal_records_info_till_end_of_wal(PG_FUNCTION_ARGS)
if (start_lsn > end_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("WAL start LSN must be less than current LSN"),
+ errmsg("WAL start LSN must be less than or equal to current LSN"),
errdetail("Current WAL LSN on the database system is at %X/%08X.",
LSN_FORMAT_ARGS(end_lsn))));
@@ -855,7 +855,7 @@ pg_get_wal_stats_till_end_of_wal(PG_FUNCTION_ARGS)
if (start_lsn > end_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("WAL start LSN must be less than current LSN"),
+ errmsg("WAL start LSN must be less than or equal to current LSN"),
errdetail("Current WAL LSN on the database system is at %X/%08X.",
LSN_FORMAT_ARGS(end_lsn))));
--
2.50.1 (Apple Git-155)
=
v4-0002-Fix-inaccurate-inclusive-bound-error-messages.patch (application/octet-stream, 5.8K) download
From 68804988701962e4fe97709ba437058373d1653c Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <lic@highgo.com>
Date: Mon, 21 Sep 2026 17:31:14 +0800
Subject: [PATCH v4 2/2] Fix inaccurate inclusive-bound error messages.
ALTER SUBSCRIPTION ... SKIP accepts a skip LSN equal to the replication
origin's progress, but its error message said that the skip LSN must be
greater than the origin LSN. Likewise, ParseVariableDouble() accepts
values at either end of its documented [min,max] range, but its
messages said that they had to be strictly inside the range.
Adjust the messages to describe the inclusive bounds accurately, and
add coverage for both cases.
Suggested-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Author: Chao Li <lic@highgo.com>
Reviewed-by: Kiran Kaki <itskkpg@gmail.com>
Discussion: https://postgr.es/m/7B8F5F12-98A5-4618-867A-904EA1334FD4@gmail.com
---
src/backend/commands/subscriptioncmds.c | 2 +-
src/bin/psql/t/001_basic.pl | 15 +++++++++++++++
src/bin/psql/variables.c | 4 ++--
src/test/regress/expected/subscription.out | 12 ++++++++++++
src/test/regress/sql/subscription.sql | 12 ++++++++++++
5 files changed, 42 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 22a61dca65d..27a5b55e964 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -2419,7 +2419,7 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt,
if (XLogRecPtrIsValid(remote_lsn) && opts.lsn < remote_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("skip WAL location (LSN %X/%08X) must be greater than origin LSN %X/%08X",
+ errmsg("skip WAL location (LSN %X/%08X) must be greater than or equal to origin LSN %X/%08X",
LSN_FORMAT_ARGS(opts.lsn),
LSN_FORMAT_ARGS(remote_lsn))));
}
diff --git a/src/bin/psql/t/001_basic.pl b/src/bin/psql/t/001_basic.pl
index 028df33ce8a..3ea0e226a36 100644
--- a/src/bin/psql/t/001_basic.pl
+++ b/src/bin/psql/t/001_basic.pl
@@ -452,6 +452,21 @@ psql_fails_like(
'\set WATCH_INTERVAL 1e500',
qr/is out of range/,
'WATCH_INTERVAL variable is out of range');
+psql_like(
+ $node,
+ '\set WATCH_INTERVAL 1000000' . "\n" . '\echo :WATCH_INTERVAL',
+ qr/^1000000$/m,
+ 'WATCH_INTERVAL accepts its upper bound');
+psql_fails_like(
+ $node,
+ '\set WATCH_INTERVAL -1',
+ qr/must be greater than or equal to 0\.00/,
+ 'WATCH_INTERVAL variable is below its lower bound');
+psql_fails_like(
+ $node,
+ '\set WATCH_INTERVAL 1000001',
+ qr/must be less than or equal to 1000000\.00/,
+ 'WATCH_INTERVAL variable is above its upper bound');
psql_like($node, '\echo :WATCH_INTERVAL',
qr/^2$/m, 'WATCH_INTERVAL variable was not altered');
diff --git a/src/bin/psql/variables.c b/src/bin/psql/variables.c
index 8060f2959cc..2c9a191a69b 100644
--- a/src/bin/psql/variables.c
+++ b/src/bin/psql/variables.c
@@ -215,14 +215,14 @@ ParseVariableDouble(const char *value, const char *name, double *result, double
if (dblval < min)
{
if (name)
- pg_log_error("invalid value \"%s\" for variable \"%s\": must be greater than %.2f",
+ pg_log_error("invalid value \"%s\" for variable \"%s\": must be greater than or equal to %.2f",
value, name, min);
return false;
}
else if (dblval > max)
{
if (name)
- pg_log_error("invalid value \"%s\" for variable \"%s\": must be less than %.2f",
+ pg_log_error("invalid value \"%s\" for variable \"%s\": must be less than or equal to %.2f",
value, name, max);
return false;
}
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index 681dc66dca6..d2ca288f9e2 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -294,6 +294,18 @@ ALTER SUBSCRIPTION regress_testsub SKIP (lsn = '0/12345');
-- ok - with lsn = NONE
ALTER SUBSCRIPTION regress_testsub SKIP (lsn = NONE);
+-- fail - LSN must not be behind the replication origin
+RESET SESSION AUTHORIZATION;
+SELECT pg_replication_origin_advance(
+ 'pg_' || (SELECT oid FROM pg_subscription WHERE subname = 'regress_testsub'),
+ '0/12346') \gset
+-- ok - LSN equal to the replication origin is accepted
+ALTER SUBSCRIPTION regress_testsub SKIP (lsn = '0/12346');
+ALTER SUBSCRIPTION regress_testsub SKIP (lsn = NONE);
+-- fail - LSN behind the replication origin is rejected
+ALTER SUBSCRIPTION regress_testsub SKIP (lsn = '0/12345');
+ERROR: skip WAL location (LSN 0/00012345) must be greater than or equal to origin LSN 0/00012346
+SET SESSION AUTHORIZATION regress_subscription_user;
-- fail
ALTER SUBSCRIPTION regress_testsub SKIP (lsn = '0/0');
ERROR: invalid WAL location (LSN): 0/0
diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql
index cfeebaf9302..0ac491a70b6 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -231,6 +231,18 @@ ALTER SUBSCRIPTION regress_testsub SKIP (lsn = '0/12345');
-- ok - with lsn = NONE
ALTER SUBSCRIPTION regress_testsub SKIP (lsn = NONE);
+-- fail - LSN must not be behind the replication origin
+RESET SESSION AUTHORIZATION;
+SELECT pg_replication_origin_advance(
+ 'pg_' || (SELECT oid FROM pg_subscription WHERE subname = 'regress_testsub'),
+ '0/12346') \gset
+-- ok - LSN equal to the replication origin is accepted
+ALTER SUBSCRIPTION regress_testsub SKIP (lsn = '0/12346');
+ALTER SUBSCRIPTION regress_testsub SKIP (lsn = NONE);
+-- fail - LSN behind the replication origin is rejected
+ALTER SUBSCRIPTION regress_testsub SKIP (lsn = '0/12345');
+SET SESSION AUTHORIZATION regress_subscription_user;
+
-- fail
ALTER SUBSCRIPTION regress_testsub SKIP (lsn = '0/0');
--
2.50.1 (Apple Git-155)
=
* Re: pg_walinspect: fix LSN validation messages and empty range handling
@ 2026-09-22 18:10 surya poondla <suryapoondla4@gmail.com>
parent: Chao Li <li.evan.chao@gmail.com>
1 sibling, 2 replies; 16+ messages in thread
From: surya poondla @ 2026-09-22 18:10 UTC (permalink / raw)
To: Chao Li <li.evan.chao@gmail.com>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>; Michael Paquier <michael@paquier.xyz>; itskkpg@gmail.com <itskkpg@gmail.com>
Hi Chao,
Thanks for the patches. I reviewed and tested v4 on master (e8a3ee5b197).
Both apply cleanly with git am, build without new
warnings, and the below things pass:
- contrib/pg_walinspect: 2/2 (pg_walinspect, oldextversions)
- src/test/regress: 239/239, including subscription
- src/bin/psql TAP: 4 files, 237 tests, including 001_basic.pl
0001 looks good to me, no comments. A few things I checked specifically:
1) All five affected messages in pg_walinspect.c are covered (lines 491,
533, 540, 839 and 858 after the patch).
Grepping the tree for the old wording across *.c, *.out, *.sql and *.sgml
turns up nothing, so no expected file or doc text is left stale.
2) The characterisation of 5c1b6628075a is accurate: it really did change
"lsn >= *curr_lsn" to "lsn > curr_lsn" and
"start_lsn >= end_lsn" to "start_lsn > *end_lsn" while only rewording the
messages.
3) pgwalinspect.sgml never states the strict relation, so no doc change is
owed here.
4) Both new wordings have precedent, which matches Bharath's preference
upthread: "must be less than or equal to" appears in
pseudorandomfuncs.c, rangetypes.c and numeric.c, and "must be greater than
or equal to" in pg_upgrade/check.c, pg_resetwal.c
and xlogrecovery.c.
0002 also looks correct to me. Both message changes match the conditions
they guard, and the two new failure-case tests pin the new
wording, so they would not pass against the old messages.
The comments below are all about the test additions rather than the fix
itself; none of them affect correctness, so please treat them as
suggestions.
1. In subscription.sql, I don't think the session-authorization is needed.
regress_subscription_user is created as LOGIN SUPERUSER at the top of the
file and is the active role
from line 190 onwards, so the session is already superuser when the new
block runs.
I removed the RESET/SET pair locally and the subscription test still
passes, so the superuser bypass over
pg_replication_origin_advance's restricted ACL is enough.
Dropping them also keeps the new test running as the subscription owner,
which is what every other SKIP test in the file does.
2. The first comment in the new block looks misplaced:
-- fail - LSN must not be behind the replication origin
RESET SESSION AUTHORIZATION;
RESET SESSION AUTHORIZATION neither fails nor has anything to do with the
origin, and the comment duplicates the one further down above the statement
it actually describes. I think it can just be removed.
3. Still in that block, pg_replication_origin_advance() returns
void, so its output is stable across runs and there is nothing for \gset to
suppress. A plain terminated statement would read more naturally and
wouldn't leave a psql variable behind.
4. Very minor: the new WATCH_INTERVAL test uses
'\set WATCH_INTERVAL 1000000' . "\n" . '\echo :WATCH_INTERVAL'
where the surrounding tests in 001_basic.pl use a literal multiline
single-quoted string.
Finally, on Bharath's question upthread about whether anything else needs
the same treatment: I went through the remaining
"must be less than" / "must be greater than" messages in the tree and each
one is consistent with the condition it guards, so I believe the
change is complete. The ones I looked at were postinit.c, postmaster.c,
partbounds.c, parse_utilcmd.c, sequence.c, wparser_def.c and pl_exec.c.
Regards,
Surya Poondla
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: pg_walinspect: fix LSN validation messages and empty range handling
@ 2026-09-22 18:41 Kiran Kaki <itskkpg@gmail.com>
parent: surya poondla <suryapoondla4@gmail.com>
1 sibling, 0 replies; 16+ messages in thread
From: Kiran Kaki @ 2026-09-22 18:41 UTC (permalink / raw)
To: surya poondla <suryapoondla4@gmail.com>; +Cc: Chao Li <li.evan.chao@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>; Michael Paquier <michael@paquier.xyz>
👍
Kiran Kaki reacted via Gmail
<https://www.google.com/gmail/about/?utm_source=gmail-in-product&utm_medium=et&utm_campaign=e...;
On Tue, Sep 22, 2026 at 11:10 AM surya poondla <suryapoondla4@gmail.com>
wrote:
> Hi Chao,
>
> Thanks for the patches. I reviewed and tested v4 on master
> (e8a3ee5b197). Both apply cleanly with git am, build without new
> warnings, and the below things pass:
> - contrib/pg_walinspect: 2/2 (pg_walinspect, oldextversions)
> - src/test/regress: 239/239, including subscription
> - src/bin/psql TAP: 4 files, 237 tests, including 001_basic.pl
>
> 0001 looks good to me, no comments. A few things I checked specifically:
> 1) All five affected messages in pg_walinspect.c are covered (lines 491,
> 533, 540, 839 and 858 after the patch).
> Grepping the tree for the old wording across *.c, *.out, *.sql and *.sgml
> turns up nothing, so no expected file or doc text is left stale.
> 2) The characterisation of 5c1b6628075a is accurate: it really did change
> "lsn >= *curr_lsn" to "lsn > curr_lsn" and
> "start_lsn >= end_lsn" to "start_lsn > *end_lsn" while only rewording the
> messages.
> 3) pgwalinspect.sgml never states the strict relation, so no doc change is
> owed here.
> 4) Both new wordings have precedent, which matches Bharath's preference
> upthread: "must be less than or equal to" appears in
> pseudorandomfuncs.c, rangetypes.c and numeric.c, and "must be greater than
> or equal to" in pg_upgrade/check.c, pg_resetwal.c
> and xlogrecovery.c.
>
> 0002 also looks correct to me. Both message changes match the conditions
> they guard, and the two new failure-case tests pin the new
> wording, so they would not pass against the old messages.
> The comments below are all about the test additions rather than the fix
> itself; none of them affect correctness, so please treat them as
> suggestions.
>
> 1. In subscription.sql, I don't think the session-authorization is needed.
>
> regress_subscription_user is created as LOGIN SUPERUSER at the top of the
> file and is the active role
> from line 190 onwards, so the session is already superuser when the new
> block runs.
> I removed the RESET/SET pair locally and the subscription test still
> passes, so the superuser bypass over
> pg_replication_origin_advance's restricted ACL is enough.
> Dropping them also keeps the new test running as the subscription owner,
> which is what every other SKIP test in the file does.
>
> 2. The first comment in the new block looks misplaced:
> -- fail - LSN must not be behind the replication origin
> RESET SESSION AUTHORIZATION;
>
> RESET SESSION AUTHORIZATION neither fails nor has anything to do with the
> origin, and the comment duplicates the one further down above the statement
> it actually describes. I think it can just be removed.
>
> 3. Still in that block, pg_replication_origin_advance() returns
> void, so its output is stable across runs and there is nothing for \gset
> to suppress. A plain terminated statement would read more naturally and
> wouldn't leave a psql variable behind.
>
> 4. Very minor: the new WATCH_INTERVAL test uses
>
> '\set WATCH_INTERVAL 1000000' . "\n" . '\echo :WATCH_INTERVAL'
> where the surrounding tests in 001_basic.pl use a literal multiline
> single-quoted string.
>
> Finally, on Bharath's question upthread about whether anything else needs
> the same treatment: I went through the remaining
> "must be less than" / "must be greater than" messages in the tree and each
> one is consistent with the condition it guards, so I believe the
> change is complete. The ones I looked at were postinit.c, postmaster.c,
> partbounds.c, parse_utilcmd.c, sequence.c, wparser_def.c and pl_exec.c.
>
> Regards,
> Surya Poondla
>
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: pg_walinspect: fix LSN validation messages and empty range handling
@ 2026-09-22 21:35 Kiran Kaki <itskkpg@gmail.com>
parent: surya poondla <suryapoondla4@gmail.com>
1 sibling, 1 reply; 16+ messages in thread
From: Kiran Kaki @ 2026-09-22 21:35 UTC (permalink / raw)
To: surya poondla <suryapoondla4@gmail.com>; +Cc: Chao Li <li.evan.chao@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>; Michael Paquier <michael@paquier.xyz>
Hi Surya, Chao
>> PFA v4:
>>
>> * 0001 unchanged
>> * 0002 added the tests that Kiran suggested
Thanks for the review, Surya. I tested v4 on the same base
(e8a3ee5b197) and results are inline with your
findings
> 1. In subscription.sql, I don't think the session-authorization is needed.
> regress_subscription_user is created as LOGIN SUPERUSER at the top of the file and is the active role
> from line 190 onwards, so the session is already superuser when the new block runs.
> I removed the RESET/SET pair locally and the subscription test still passes, so the superuser bypass over
> pg_replication_origin_advance's restricted ACL is enough.
> Dropping them also keeps the new test running as the subscription owner, which is what every other SKIP test in the file does.
Agree on this!
> 2. The first comment in the new block looks misplaced:
> -- fail - LSN must not be behind the replication origin
> RESET SESSION AUTHORIZATION;
>
> RESET SESSION AUTHORIZATION neither fails nor has anything to do with the origin, and the comment duplicates the one further down above the statement it actually describes. I think it can just be removed.
Agree on this!
> 3. Still in that block, pg_replication_origin_advance() returns
> void, so its output is stable across runs and there is nothing for \gset to suppress. A plain terminated statement would read more naturally and wouldn't leave a psql variable behind.
Agree on 3. The \gset here is only hiding an empty one-row result, and
a plain statement says what it means. Worth noting it adds five lines to
the expected file, which seems a fair trade.
> 4. Very minor: the new WATCH_INTERVAL test uses
>
> '\set WATCH_INTERVAL 1000000' . "\n" . '\echo :WATCH_INTERVAL'
> where the surrounding tests in 001_basic.pl use a literal multiline single-quoted string.
Point 4 is my mistake, not Chao's. That concatenation came from the
snippet I sent upthread and he may have used it as-is. Sorry for the noise.
Thanks,
Kiran Kaki.
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: pg_walinspect: fix LSN validation messages and empty range handling
@ 2026-09-23 01:11 Chao Li <li.evan.chao@gmail.com>
parent: Kiran Kaki <itskkpg@gmail.com>
0 siblings, 1 reply; 16+ messages in thread
From: Chao Li @ 2026-09-23 01:11 UTC (permalink / raw)
To: Kiran Kaki <itskkpg@gmail.com>; +Cc: surya poondla <suryapoondla4@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>; Michael Paquier <michael@paquier.xyz>
> On Sep 23, 2026, at 05:35, Kiran Kaki <itskkpg@gmail.com> wrote:
>
> Hi Surya, Chao
>
>>> PFA v4:
>>>
>>> * 0001 unchanged
>>> * 0002 added the tests that Kiran suggested
>
> Thanks for the review, Surya. I tested v4 on the same base
> (e8a3ee5b197) and results are inline with your
> findings
>
>> 1. In subscription.sql, I don't think the session-authorization is needed.
>> regress_subscription_user is created as LOGIN SUPERUSER at the top of the file and is the active role
>> from line 190 onwards, so the session is already superuser when the new block runs.
>> I removed the RESET/SET pair locally and the subscription test still passes, so the superuser bypass over
>> pg_replication_origin_advance's restricted ACL is enough.
>> Dropping them also keeps the new test running as the subscription owner, which is what every other SKIP test in the file does.
>
> Agree on this!
Fixed
>
>> 2. The first comment in the new block looks misplaced:
>> -- fail - LSN must not be behind the replication origin
>> RESET SESSION AUTHORIZATION;
>>
>> RESET SESSION AUTHORIZATION neither fails nor has anything to do with the origin, and the comment duplicates the one further down above the statement it actually describes. I think it can just be removed.
>
> Agree on this!
My bad. I initially put the comment before the code block, but later moved it to before the exact failure statement but forgot to delete that one.
>
>> 3. Still in that block, pg_replication_origin_advance() returns
>> void, so its output is stable across runs and there is nothing for \gset to suppress. A plain terminated statement would read more naturally and wouldn't leave a psql variable behind.
>
> Agree on 3. The \gset here is only hiding an empty one-row result, and
> a plain statement says what it means. Worth noting it adds five lines to
> the expected file, which seems a fair trade.
Fixed.
>
>> 4. Very minor: the new WATCH_INTERVAL test uses
>>
>> '\set WATCH_INTERVAL 1000000' . "\n" . '\echo :WATCH_INTERVAL'
>> where the surrounding tests in 001_basic.pl use a literal multiline single-quoted string.
>
> Point 4 is my mistake, not Chao's. That concatenation came from the
> snippet I sent upthread and he may have used it as-is. Sorry for the noise.
>
>
Fixed.
PFA v5:
* 0001: only added Surya as a reviewed in the commit message
* 0002: addressed Surya’s review comments
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Attachments: download all (2 files, .tar.gz)
v5-0001-Fix-input-LSN-validation-error-messages-in-pg_wal.patch (application/octet-stream, 6.4K) download
From 73724f49e23e42aa4f20aee05a89f5cf203f9fab Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <lic@highgo.com>
Date: Sun, 20 Sep 2026 09:39:56 +0800
Subject: [PATCH v5 1/2] Fix input LSN validation error messages in
pg_walinspect.
Commit 5c1b6628075a changed the input LSN validation checks to allow an input
LSN equal to the current LSN, and a start LSN equal to the end LSN. However,
the corresponding error messages continued to say that the LSNs had to be
less than the current LSN or the end LSN.
Fix this oversight by changing those messages from "must be less than" to
"must be less than or equal to", matching the actual validation conditions.
Similar wording is used elsewhere in the code.
Not back-patched, as the fix is not critical and the inaccuracy went unnoticed
for years.
Author: Chao Li <lic@highgo.com>
Reviewed-by: Kiran Kaki <itskkpg@gmail.com>
Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Surya Poondla <suryapoondla4@gmail.com>
Discussion: https://postgr.es/m/7B8F5F12-98A5-4618-867A-904EA1334FD4@gmail.com
---
contrib/pg_walinspect/expected/oldextversions.out | 4 ++--
contrib/pg_walinspect/expected/pg_walinspect.out | 14 +++++++-------
contrib/pg_walinspect/pg_walinspect.c | 10 +++++-----
3 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/contrib/pg_walinspect/expected/oldextversions.out b/contrib/pg_walinspect/expected/oldextversions.out
index 89953af3d0e..fd65ed149c6 100644
--- a/contrib/pg_walinspect/expected/oldextversions.out
+++ b/contrib/pg_walinspect/expected/oldextversions.out
@@ -43,9 +43,9 @@ SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_stats_till_end_of_wal(:'wal_lsn1');
-- Failures with start LSNs.
SELECT * FROM pg_get_wal_records_info_till_end_of_wal('FFFFFFFF/FFFFFFFF');
-ERROR: WAL start LSN must be less than current LSN
+ERROR: WAL start LSN must be less than or equal to current LSN
SELECT * FROM pg_get_wal_stats_till_end_of_wal('FFFFFFFF/FFFFFFFF');
-ERROR: WAL start LSN must be less than current LSN
+ERROR: WAL start LSN must be less than or equal to current LSN
-- Move to new version 1.1.
ALTER EXTENSION pg_walinspect UPDATE TO '1.1';
-- List what version 1.1 contains.
diff --git a/contrib/pg_walinspect/expected/pg_walinspect.out b/contrib/pg_walinspect/expected/pg_walinspect.out
index f955ff5d3c5..b9abeeebdfd 100644
--- a/contrib/pg_walinspect/expected/pg_walinspect.out
+++ b/contrib/pg_walinspect/expected/pg_walinspect.out
@@ -29,14 +29,14 @@ SELECT * FROM pg_get_wal_block_info('0/0', :'wal_lsn1');
ERROR: could not read WAL at LSN 0/00000000
-- Start LSN > End LSN.
SELECT * FROM pg_get_wal_records_info(:'wal_lsn2', :'wal_lsn1');
-ERROR: WAL start LSN must be less than end LSN
+ERROR: WAL start LSN must be less than or equal to end LSN
SELECT * FROM pg_get_wal_stats(:'wal_lsn2', :'wal_lsn1');
-ERROR: WAL start LSN must be less than end LSN
+ERROR: WAL start LSN must be less than or equal to end LSN
SELECT * FROM pg_get_wal_block_info(:'wal_lsn2', :'wal_lsn1');
-ERROR: WAL start LSN must be less than end LSN
+ERROR: WAL start LSN must be less than or equal to end LSN
-- LSNs with the highest value possible.
SELECT * FROM pg_get_wal_record_info('FFFFFFFF/FFFFFFFF');
-ERROR: WAL input LSN must be less than current LSN
+ERROR: WAL input LSN must be less than or equal to current LSN
-- Success with end LSNs.
SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_records_info(:'wal_lsn1', 'FFFFFFFF/FFFFFFFF');
ok
@@ -58,11 +58,11 @@ SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_block_info(:'wal_lsn1', 'FFFFFFFF/FFF
-- Failures with start LSNs.
SELECT * FROM pg_get_wal_records_info('FFFFFFFF/FFFFFFFE', 'FFFFFFFF/FFFFFFFF');
-ERROR: WAL start LSN must be less than current LSN
+ERROR: WAL start LSN must be less than or equal to current LSN
SELECT * FROM pg_get_wal_stats('FFFFFFFF/FFFFFFFE', 'FFFFFFFF/FFFFFFFF');
-ERROR: WAL start LSN must be less than current LSN
+ERROR: WAL start LSN must be less than or equal to current LSN
SELECT * FROM pg_get_wal_block_info('FFFFFFFF/FFFFFFFE', 'FFFFFFFF/FFFFFFFF');
-ERROR: WAL start LSN must be less than current LSN
+ERROR: WAL start LSN must be less than or equal to current LSN
-- ===================================================================
-- Tests for all function executions
-- ===================================================================
diff --git a/contrib/pg_walinspect/pg_walinspect.c b/contrib/pg_walinspect/pg_walinspect.c
index a172f9e2b40..6edd6f8cb35 100644
--- a/contrib/pg_walinspect/pg_walinspect.c
+++ b/contrib/pg_walinspect/pg_walinspect.c
@@ -488,7 +488,7 @@ pg_get_wal_record_info(PG_FUNCTION_ARGS)
if (lsn > curr_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("WAL input LSN must be less than current LSN"),
+ errmsg("WAL input LSN must be less than or equal to current LSN"),
errdetail("Current WAL LSN on the database system is at %X/%08X.",
LSN_FORMAT_ARGS(curr_lsn))));
@@ -530,14 +530,14 @@ ValidateInputLSNs(XLogRecPtr start_lsn, XLogRecPtr *end_lsn)
if (start_lsn > curr_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("WAL start LSN must be less than current LSN"),
+ errmsg("WAL start LSN must be less than or equal to current LSN"),
errdetail("Current WAL LSN on the database system is at %X/%08X.",
LSN_FORMAT_ARGS(curr_lsn))));
if (start_lsn > *end_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("WAL start LSN must be less than end LSN")));
+ errmsg("WAL start LSN must be less than or equal to end LSN")));
if (*end_lsn > curr_lsn)
*end_lsn = curr_lsn;
@@ -836,7 +836,7 @@ pg_get_wal_records_info_till_end_of_wal(PG_FUNCTION_ARGS)
if (start_lsn > end_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("WAL start LSN must be less than current LSN"),
+ errmsg("WAL start LSN must be less than or equal to current LSN"),
errdetail("Current WAL LSN on the database system is at %X/%08X.",
LSN_FORMAT_ARGS(end_lsn))));
@@ -855,7 +855,7 @@ pg_get_wal_stats_till_end_of_wal(PG_FUNCTION_ARGS)
if (start_lsn > end_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("WAL start LSN must be less than current LSN"),
+ errmsg("WAL start LSN must be less than or equal to current LSN"),
errdetail("Current WAL LSN on the database system is at %X/%08X.",
LSN_FORMAT_ARGS(end_lsn))));
--
2.50.1 (Apple Git-155)
=
v5-0002-Fix-inaccurate-inclusive-bound-error-messages.patch (application/octet-stream, 5.6K) download
From a2912be11f0f4e9ec70e3feae9083b809a843f77 Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <lic@highgo.com>
Date: Mon, 21 Sep 2026 17:31:14 +0800
Subject: [PATCH v5 2/2] Fix inaccurate inclusive-bound error messages.
ALTER SUBSCRIPTION ... SKIP accepts a skip LSN equal to the replication
origin's progress, but its error message said that the skip LSN must be
greater than the origin LSN. Likewise, ParseVariableDouble() accepts
values at either end of its documented [min,max] range, but its
messages said that they had to be strictly inside the range.
Adjust the messages to describe the inclusive bounds accurately, and
add coverage for both cases.
Suggested-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Author: Chao Li <lic@highgo.com>
Reviewed-by: Kiran Kaki <itskkpg@gmail.com>
Reviewed-by: Surya Poondla <suryapoondla4@gmail.com>
Discussion: https://postgr.es/m/7B8F5F12-98A5-4618-867A-904EA1334FD4@gmail.com
---
src/backend/commands/subscriptioncmds.c | 2 +-
src/bin/psql/t/001_basic.pl | 16 ++++++++++++++++
src/bin/psql/variables.c | 4 ++--
src/test/regress/expected/subscription.out | 14 ++++++++++++++
src/test/regress/sql/subscription.sql | 9 +++++++++
5 files changed, 42 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 22a61dca65d..27a5b55e964 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -2419,7 +2419,7 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt,
if (XLogRecPtrIsValid(remote_lsn) && opts.lsn < remote_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("skip WAL location (LSN %X/%08X) must be greater than origin LSN %X/%08X",
+ errmsg("skip WAL location (LSN %X/%08X) must be greater than or equal to origin LSN %X/%08X",
LSN_FORMAT_ARGS(opts.lsn),
LSN_FORMAT_ARGS(remote_lsn))));
}
diff --git a/src/bin/psql/t/001_basic.pl b/src/bin/psql/t/001_basic.pl
index 028df33ce8a..a36f6ba5b8e 100644
--- a/src/bin/psql/t/001_basic.pl
+++ b/src/bin/psql/t/001_basic.pl
@@ -452,6 +452,22 @@ psql_fails_like(
'\set WATCH_INTERVAL 1e500',
qr/is out of range/,
'WATCH_INTERVAL variable is out of range');
+psql_like(
+ $node,
+ '\set WATCH_INTERVAL 1000000
+\echo :WATCH_INTERVAL',
+ qr/^1000000$/m,
+ 'WATCH_INTERVAL accepts its upper bound');
+psql_fails_like(
+ $node,
+ '\set WATCH_INTERVAL -1',
+ qr/must be greater than or equal to 0\.00/,
+ 'WATCH_INTERVAL variable is below its lower bound');
+psql_fails_like(
+ $node,
+ '\set WATCH_INTERVAL 1000001',
+ qr/must be less than or equal to 1000000\.00/,
+ 'WATCH_INTERVAL variable is above its upper bound');
psql_like($node, '\echo :WATCH_INTERVAL',
qr/^2$/m, 'WATCH_INTERVAL variable was not altered');
diff --git a/src/bin/psql/variables.c b/src/bin/psql/variables.c
index 8060f2959cc..2c9a191a69b 100644
--- a/src/bin/psql/variables.c
+++ b/src/bin/psql/variables.c
@@ -215,14 +215,14 @@ ParseVariableDouble(const char *value, const char *name, double *result, double
if (dblval < min)
{
if (name)
- pg_log_error("invalid value \"%s\" for variable \"%s\": must be greater than %.2f",
+ pg_log_error("invalid value \"%s\" for variable \"%s\": must be greater than or equal to %.2f",
value, name, min);
return false;
}
else if (dblval > max)
{
if (name)
- pg_log_error("invalid value \"%s\" for variable \"%s\": must be less than %.2f",
+ pg_log_error("invalid value \"%s\" for variable \"%s\": must be less than or equal to %.2f",
value, name, max);
return false;
}
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index 681dc66dca6..f2143bcd029 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -294,6 +294,20 @@ ALTER SUBSCRIPTION regress_testsub SKIP (lsn = '0/12345');
-- ok - with lsn = NONE
ALTER SUBSCRIPTION regress_testsub SKIP (lsn = NONE);
+SELECT pg_replication_origin_advance(
+ 'pg_' || (SELECT oid FROM pg_subscription WHERE subname = 'regress_testsub'),
+ '0/12346');
+ pg_replication_origin_advance
+-------------------------------
+
+(1 row)
+
+-- ok - LSN equal to the replication origin is accepted
+ALTER SUBSCRIPTION regress_testsub SKIP (lsn = '0/12346');
+ALTER SUBSCRIPTION regress_testsub SKIP (lsn = NONE);
+-- fail - LSN behind the replication origin is rejected
+ALTER SUBSCRIPTION regress_testsub SKIP (lsn = '0/12345');
+ERROR: skip WAL location (LSN 0/00012345) must be greater than or equal to origin LSN 0/00012346
-- fail
ALTER SUBSCRIPTION regress_testsub SKIP (lsn = '0/0');
ERROR: invalid WAL location (LSN): 0/0
diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql
index cfeebaf9302..0150f9bd044 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -231,6 +231,15 @@ ALTER SUBSCRIPTION regress_testsub SKIP (lsn = '0/12345');
-- ok - with lsn = NONE
ALTER SUBSCRIPTION regress_testsub SKIP (lsn = NONE);
+SELECT pg_replication_origin_advance(
+ 'pg_' || (SELECT oid FROM pg_subscription WHERE subname = 'regress_testsub'),
+ '0/12346');
+-- ok - LSN equal to the replication origin is accepted
+ALTER SUBSCRIPTION regress_testsub SKIP (lsn = '0/12346');
+ALTER SUBSCRIPTION regress_testsub SKIP (lsn = NONE);
+-- fail - LSN behind the replication origin is rejected
+ALTER SUBSCRIPTION regress_testsub SKIP (lsn = '0/12345');
+
-- fail
ALTER SUBSCRIPTION regress_testsub SKIP (lsn = '0/0');
--
2.50.1 (Apple Git-155)
=
* Re: pg_walinspect: fix LSN validation messages and empty range handling
@ 2026-09-23 23:13 Kiran Kaki <itskkpg@gmail.com>
parent: Chao Li <li.evan.chao@gmail.com>
0 siblings, 1 reply; 16+ messages in thread
From: Kiran Kaki @ 2026-09-23 23:13 UTC (permalink / raw)
To: Chao Li <li.evan.chao@gmail.com>; +Cc: surya poondla <suryapoondla4@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>; Michael Paquier <michael@paquier.xyz>
Hi Chao
>
> PFA v5:
>
> * 0001: only added Surya as a reviewed in the commit message
> * 0002: addressed Surya’s review comments
>
Thanks for addressing the review comments . All four issues are fixed in v5.
Reviewed and tested on master (d39fda1cc4):
- Both patches apply cleanly.
- Build succeeds.
- pg_walinspect tests pass.
- psql tests pass.
- Core regression tests pass.
- Manually verified: ERROR: skip WAL location (LSN 0/00012345) must be
greater than or equal to origin LSN 0/00012346
Looks ready for a committer.
Regards,
Kiran Kaki
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: pg_walinspect: fix LSN validation messages and empty range handling
@ 2026-09-24 01:03 Xuneng Zhou <xunengzhou@gmail.com>
parent: Kiran Kaki <itskkpg@gmail.com>
0 siblings, 1 reply; 16+ messages in thread
From: Xuneng Zhou @ 2026-09-24 01:03 UTC (permalink / raw)
To: Kiran Kaki <itskkpg@gmail.com>; +Cc: Chao Li <li.evan.chao@gmail.com>; surya poondla <suryapoondla4@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>; Michael Paquier <michael@paquier.xyz>
On Thu, Sep 24, 2026 at 7:14 AM Kiran Kaki <itskkpg@gmail.com> wrote:
> Hi Chao
>
> >
> > PFA v5:
> >
> > * 0001: only added Surya as a reviewed in the commit message
> > * 0002: addressed Surya’s review comments
> >
>
> Thanks for addressing the review comments . All four issues are fixed in
> v5.
>
> Reviewed and tested on master (d39fda1cc4):
>
> - Both patches apply cleanly.
> - Build succeeds.
> - pg_walinspect tests pass.
> - psql tests pass.
> - Core regression tests pass.
> - Manually verified: ERROR: skip WAL location (LSN 0/00012345) must be
> greater than or equal to origin LSN 0/00012346
>
> Looks ready for a committer.
>
+1. v5 LGTM.
--
Regards,
Xuneng Zhou
HighGo Software Co., Ltd.
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: pg_walinspect: fix LSN validation messages and empty range handling
@ 2026-09-24 18:29 surya poondla <suryapoondla4@gmail.com>
parent: Xuneng Zhou <xunengzhou@gmail.com>
0 siblings, 0 replies; 16+ messages in thread
From: surya poondla @ 2026-09-24 18:29 UTC (permalink / raw)
To: Xuneng Zhou <xunengzhou@gmail.com>; Chao Li <li.evan.chao@gmail.com>; +Cc: Kiran Kaki <itskkpg@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>; Michael Paquier <michael@paquier.xyz>
Hi Chao,
Thanks for the v5 patch (all the points that were earlier raised were
addressed). Both patches apply with git am, the build is clean with no new
warnings.
I also checked that advancing the origin doesn't leak into the rest of
subscription.sql: no later test sets a skip LSN with a valid location, and
the
new block leaves subskiplsn reset to NONE, so nothing downstream depends on
the origin position.
No further comments from me, v5 looks ready for a committer. You can create
a commitfest entry for this.
Regards,
Surya Poondla
^ permalink raw reply [nested|flat] 16+ messages in thread
end of thread, other threads:[~2026-09-24 18:29 UTC | newest]
Thread overview: 16+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-09-20 03:08 pg_walinspect: fix LSN validation messages and empty range handling Chao Li <li.evan.chao@gmail.com>
2026-09-20 18:40 ` Kiran Kaki <itskkpg@gmail.com>
2026-09-21 06:56 ` Chao Li <li.evan.chao@gmail.com>
2026-09-21 08:55 ` Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2026-09-21 15:28 ` Kiran Kaki <itskkpg@gmail.com>
2026-09-21 22:50 ` Chao Li <li.evan.chao@gmail.com>
2026-09-22 02:06 ` Kiran Kaki <itskkpg@gmail.com>
2026-09-22 05:20 ` Chao Li <li.evan.chao@gmail.com>
2026-09-21 15:29 ` Kiran Kaki <itskkpg@gmail.com>
2026-09-22 18:10 ` surya poondla <suryapoondla4@gmail.com>
2026-09-22 18:41 ` Kiran Kaki <itskkpg@gmail.com>
2026-09-22 21:35 ` Kiran Kaki <itskkpg@gmail.com>
2026-09-23 01:11 ` Chao Li <li.evan.chao@gmail.com>
2026-09-23 23:13 ` Kiran Kaki <itskkpg@gmail.com>
2026-09-24 01:03 ` Xuneng Zhou <xunengzhou@gmail.com>
2026-09-24 18:29 ` surya poondla <suryapoondla4@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox