Hi Chao, Thanks for v4. The test fix and the doc sentence look good, and CFBot is green now, including macOS.
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 position all look correct to me. The tie-breaks and the neighbour scan only ever widen the range, which is the safe direction. 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 user sees 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 falls back to the current WAL position.) 2. The docs say "Unlike the other pg_walinspect functions, this function may inspect retained WAL from the current timeline's history 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. 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. With these addressed, I think the patch is ready for committer. Regards, Surya Poondla
