Hi JoongHyuk, Thank you for v3. I tested it on master (87524d0), with both patches applied.
1. TAP tests: t/ https://www.google.com/url?q=http://003_recovery_targets.pl&source=gmail&ust=1794638038862000&sa=E and t/ https://www.google.com/url?q=http://005_replay_delay.pl&source=gmail&ust=1794638038862000&sa=E pass (Files=2, Tests=36, PASS). 2. Manual PITR. Before each command I inserted a row, then ran recovery_target_time restores for five commands: DROP DATABASE, CREATE DATABASE (FILE_COPY), ALTER DATABASE ... SET TABLESPACE, CREATE TABLESPACE and DROP TABLESPACE. For each one I tried these targets: - the exact boundary time, with recovery_target_inclusive = on; - the same, with inclusive = off; - the statement time from log_statement; - the time of the previous commit. In all of them recovery logged "recovery stopping before recovery boundary", and the cluster came up clean, with the command not applied. So the >= check now works the same way for both inclusive values. Thanks for fixing that. 3. The docs under recovery_target_time read clearly to me. 4. As expected from your point 1, a target between the boundary and the commit (I used boundary + 1 microsecond) still stops before the commit. In that case I saw: - DROP DATABASE: the database stays in the catalog with datconnlimit -2 (INVALID), and connecting to it fails. - CREATE DATABASE (FILE_COPY): an orphan directory under base/, with no catalog row. - CREATE TABLESPACE: an orphan directory, with no catalog row. - ALTER DATABASE SET TABLESPACE and DROP TABLESPACE: no visible problem. The window was about 20 microseconds for the tablespace commands, but about 93 ms for DROP DATABASE and about 970 ms for CREATE DATABASE on my small test. I understand keeping this out of scope. Maybe a short sentence in the docs could say that a target falling inside that window can still leave this state, so users pick a time before the command. Thanks again for working on this. Regards, João Rito On Thu, Nov 12, 2026 08:58 AM, JoongHyuk Shin <[email protected]> wrote: > Hi João, > > Thanks for testing v2 and for the detailed report. v3 is attached. > > On Wed, Nov 11, 2026 at 11:27 PM João Rito <[email protected]> > wrote: > > 1. A time target between the boundary record and the commit record > > still breaks the database. > > I don't plan to document this case. I'd expect an operator to take the > target from the statement's log_statement line, or from the last commit > before the command with recovery_target_inclusive = on, and with v3 both > stop before the command. A target that falls between the boundary record > and the commit behaves as it does on master, and covering it is not a > goal of this patch. > > > 2. With recovery_target_inclusive = on and a target exactly equal to > > the boundary time, recovery does not stop at the boundary (the test > > is boundary_time > target), replays the file operation, and then > > stops before the commit because the commit time is later. > > Agreed. The boundary record is not a transaction that > recovery_target_inclusive could include. > > > 3. 0001 does not touch the documentation. The recovery_target_time > > and recovery_target_inclusive text in config.sgml (or the PITR > > chapter) does not mention that recovery now also stops before these > > commands, nor that xid and LSN targets are not covered (the xid > > case Shihao reported). > > The recovery_target_inclusive entry is unchanged, for the reason given > in point 2. For xid and LSN targets I kept the docs as they are. As I > wrote to Shihao, the xid and LSN cases are not specific to this patch, > since master gives the same result. They are also not limited to > database commands. With recovery_target_xid set to a CREATE TABLE > transaction and recovery_target_inclusive = off, the table's file is > left behind. > > > 4. Small thing about messages. "recovery stopping before recovery > > boundary, time ..." and the pg_waldump line do not say which > > command the boundary belongs to > > Thanks for the suggestion. I kept the record to the time stamp, which > is all the stop check and the apply delay use. If others find the > command type useful, I can add it. >
