[bug fix] PITR corrupts the database cluster

Hello,

I've encountered a bug of PITR that corrupts the database.  I'm willing to 
submit the patch to fix it, but I'm wondering what approach is appropriate. 
Could you give me your opinions?

[Problem]
I cannot connect to the database after performing the following steps:

1. CREATE DATABASE mydb;
2. Take a base backup with pg_basebackup.
3. DROP DATABASE mydb;
4. Shutdown the database server with "pg_ctl stop".
5. Recover the database cluster to the point where the base backup 
completed, i.e., before dropping mydb.  The contents of recovery.conf is:
restore_command = 'cp /arc/dir/%f %p'
recovery_target_timeline = 'latest'
recovery_target_time = 'STOP TIME recorded in the backup history file which 
was created during base backup'

I expected to be able to connect to mydb because I recovered to the point 
before dropping mydb.  However, I cannot connect to mydb because the 
directory for mydb does not exist.  The entry for mydb exists in 
pg_database.


[Cause]
DROP DATABASE emits the below WAL records:

1. System catalog changes including deletion of a tuple for mydb in 
pg_database
2. Deletion of directories for the database
3. Transaction commit

During recovery, postgres replays 1 and 2.  It ends the recovery when it 
notices that the time recorded in commit record (3 above) is later than the 
recovery target time.  The commit record is not replayed, thus the system 
catalog changes are virtually undone.

The problem is that 2 is replayed.  This deletes the directory for the 
database although the transaction is not committed.


[How to fix]
There are two approaches.  Which do you think is the way to go?

<Approach 1>
During recovery, when the WAL record for directory deletion is found, just 
record that fact for later replay (in a hash table keyed by xid).  When the 
corresponding transaction commit record is found, replay the directory 
deletion record.

<Approach 2>
Like the DROP TABLE/INDEX case, piggyback the directory deletion record on 
the transaction commit record, and eliminate the directory deletion record 
altogether.

I think we need to take approach 1 even when we also does 2, because 1 is 
necessary when the backup and archive WAL are already taken with the current 
PostgreSQL anyway.


Regards
MauMau