Hi ChangAo, I tested v1 of this patch.
Environment: - PostgreSQL master (commit 9b43e6793b0) - Ubuntu 24.04, gcc 13.3.0, x86_64 - Configured with --enable-cassert --enable-debug --enable-tap-tests The patch applies cleanly and builds without new warnings. Testing performed: - make check: PASS - make check-world: PASS - make -C src/test/recovery check: PASS (599 tests) In particular, t/043_no_contrecord_switch.pl and t/039_end_of_wal.pl both pass, which exercise the XLOG_SWITCH boundary handling. Manual verification: I ran pg_switch_wal() twice with some activity in between and observed that the returned LSN and subsequent pg_current_wal_lsn() values are consistent with segment boundaries and page headers: SELECT pg_switch_wal(); -- 0/0178FE38 SELECT pg_current_wal_lsn(); -- 0/02000000 (new segment) -- ... some DDL and INSERT ... SELECT pg_switch_wal(); -- 0/02026528 SELECT pg_current_wal_lsn(); -- 0/03000060 (new segment + 0x60 -- == SizeOfXLogLongPHD) The 0x60 offset matches the long page header size, which is what the original code was computing via the explicit SizeOfXLogLongPHD / SizeOfXLogShortPHD branches. The refactored version using XLogBytePosToEndRecPtr(XLogRecPtrToBytePos(StartPos) + MAXALIGN(SizeOfXLogRecord)) produces the same result while matching the pattern used elsewhere in XLogInsertRecord(). One question: The original code did not apply MAXALIGN() to SizeOfXLogRecord before adding it. In practice SizeOfXLogRecord is likely already MAXALIGN'd (given typical record header layout), but could you confirm whether MAXALIGN() here is a correctness fix, a defensive no-op, or something that requires a separate note in the commit message? Otherwise the change looks good to me, and I think it's a reasonable cleanup. Tested-by: Jihyun Bahn <[email protected]> Regards, Jihyun Bahn
