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

From: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>
To: Kiran Kaki <itskkpg(at)gmail(dot)com>
Cc: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Michael Paquier <michael(at)paquier(dot)xyz>
Subject: Re: pg_walinspect: fix LSN validation messages and empty range handling
Date: 2026-09-22 05:20:08
Message-ID: 1D4D684B-C1EC-4630-9794-F13950C9B15F@gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On Sep 22, 2026, at 10:06, Kiran Kaki <itskkpg(at)gmail(dot)com> wrote:
>
> Hi @Chao Li
>
> On Mon, Sep 21, 2026 at 3:51 PM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)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/

Attachment Content-Type Size
v4-0001-Fix-input-LSN-validation-error-messages-in-pg_wal.patch application/octet-stream 6.4 KB
v4-0002-Fix-inaccurate-inclusive-bound-error-messages.patch application/octet-stream 5.8 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2026-09-22 05:28:05 Re: Add counted_by attribute
Previous Message Tristan Partin 2026-09-22 05:13:43 Re: Stop asserting that Unicode normalization consumes its whole input