> 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/
