> On Sep 23, 2026, at 05:35, Kiran Kaki <[email protected]> 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/
v5-0001-Fix-input-LSN-validation-error-messages-in-pg_wal.patch
Description: Binary data
v5-0002-Fix-inaccurate-inclusive-bound-error-messages.patch
Description: Binary data
