Re: [PATCH] Add recovery boundary WAL record for database and tablespace commands

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 <sjh910805(at)gmail(dot)com> 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 <joaovictor(dot)rito(at)gmail(dot)com>
> 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.
>