> On Oct 10, 2026, at 07:18, surya poondla <suryapoondla4@gmail.com> wrote:
>
> Hi Chao,
>
> Thanks for v4. The test fix and the doc sentence look good, and CFBot is green now, including macOS.
Thank you very much for the follow up reviewing.
>
> I did a full read-through of v4, especially the boundary logic in 0002.
> The binary search, the outer anchors, the extra segment scanned on each side, and the fallback to the current WAL
positionall look correct to
> me. The tie-breaks and the neighbour scan only ever widen the range, which is the safe direction.
Cool!
>
> I have a few small comments:
> 1. pg_get_wal_records_info(start_lsn, end_lsn) includes the start-anchor record but not the end-anchor one.
> end_lsn is the start LSN of the end-anchor record, and pg_get_wal_records_info() only returns records that end at or
before
> end_lsn. So if the transaction of interest committed just after the window, its COMMIT can be the end anchor, the
usersees its changes but
> not its commit, and might conclude it never committed. I wouldn't change end_lsn, since pointing
pg_get_wal_record_info()at it to see the anchor
> is useful, but a sentence in the docs would help. (This only applies when an end anchor is found, not when end_lsn
fallsback to the current WAL position.)
>
This is a good point. Since the requested time range is likely to be only a rough estimate, users may generally be more
interestedin the surrounding WAL than in the exact endpoint. But I agree that this can sometimes matter.
I considered changing end_lsn to the end-anchor record's ending LSN, but then pg_get_wal_record_info(end_lsn) could no
longerbe used to inspect the end-anchor record directly. Therefore, I added a tip to the doc explaining this situation
andhow to include the end-anchor record.
> 2. The docs say "Unlike the other pg_walinspect functions, this function may inspect retained WAL from the current
timeline'shistory as well as
> the current timeline." I don't think that's accurate. The existing functions read through
read_local_xlog_page_no_wait(),and
> XLogReadDetermineTimeline() already follows the timeline history, choosing each segment's timeline with
tliOfPointInHistory(),the same
> rule GetWalTimeSegments() uses. I'd drop the sentence.
Agreed.
>
> 3. Nit: "No retained WAL segment contains a timestamped record" is
> reported with ERRCODE_INVALID_PARAMETER_VALUE, but nothing is wrong with
> the arguments. ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE, which the patch
> already uses for "no retained WAL segments are available", seems a better fit.
Agreed.
>
> With these addressed, I think the patch is ready for committer.
>
> Regards,
> Surya Poondla
PFA v5: 0001 unchanged; 0002 addressed the new comments.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
