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

> On Sep 23, 2026, at 05:35, Kiran Kaki <itskkpg(at)gmail(dot)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/