log_object_drops: logged LSN is the commit record END, but recovery compares record START — logged LSN can never restore before the drop

[NikolayS]

Summary

The log_object_drops patch (v5) logs the end LSN of the commit record. All recovery_target_lsn comparisons in recovery are made against the record's start LSN. Therefore the logged LSN can never be used to stop before the drop, with any value of recovery_target_inclusive.

This is a design-level correctness bug, not a documentation gap. The feature as currently shipped does not do the thing it exists to do.

Evidence (verified against master, 2026-07-15)

What v5 logs — XactLogCommitRecord() returns the result of XLogInsert(), which is the EndPos (the position after the record):

/* src/backend/access/transam/xact.c */
commit_lsn = XactLogCommitRecord(GetCurrentTransactionStopTimestamp(),
                    nchildren, children, nrels, rels,
                    ...

What recovery compares — recoveryStopsBefore() tests the record's start (ReadRecPtr):

/* src/backend/access/transam/xlogrecovery.c:2580-2582 */
if (recoveryTarget == RECOVERY_TARGET_LSN &&
    !recoveryTargetInclusive &&
    record->ReadRecPtr >= recoveryTargetLSN)

recoveryStopsAfter() compares against record start as well (xlogrecovery.c:2748).

Why this breaks

Let the DROP's commit record span [start, end). v5 logs end.

• recovery_target_lsn = end, recovery_target_inclusive = off: the commit record's own ReadRecPtr is start, and start < end, so the stop-before test does not fire on it — the commit record is replayed (the table is dropped). The test then fires on the next record, whose start is >= end. Net effect: recovery stops after the drop.
• recovery_target_inclusive = on: stops after the target. Also after the drop.

Either way the table is gone. There is no setting of recovery_target_inclusive that recovers the table.

Fix options

1. Log ProcLastRecPtr instead — the start of the last record inserted by this backend (extern PGDLLIMPORT XLogRecPtr ProcLastRecPtr;, src/include/access/xlog.h:33). With a start LSN, recovery_target_lsn = <start> + recovery_target_inclusive = off correctly stops before the commit record.
2. Log the XID as well, and recommend recovery_target_xid = <xid> with recovery_target_inclusive = off. This sidesteps LSN start/end arithmetic entirely and is arguably the friendlier DBA-facing recipe.

Recommend doing both: the XID recipe is what a human should copy-paste; the correct start LSN is what tooling should consume.

Relationship to the other known issues

Kirk Wolak's list from the 2026-07-15 session:

1. Format-string bug (errmsg wants 4x %X but one LSN_FORMAT_ARGS is passed) — unaffected, fix as-is.
2. DROP DATABASE still uses GetXLogInsertRecPtr() — the deferral to commit time is right, but the deferred value must be the record start, not XactLogCommitRecord()'s return value.
3. Skip TEMP — unaffected.
4. "Document PITR use (recovery_target_inclusive)" — insufficient on its own. Documenting the incantation does not help when no value of the setting produces the advertised behaviour. Item 4 was a symptom of this bug.

Test implication

A TAP test that asserts "an LSN was logged" cannot catch this. The test must assert the logged LSN actually restores the dropped table — i.e. run a real PITR to the logged target and check the table is present. Anything weaker passes against a broken feature.

Refs

• Thread: https://www.postgresql.org/message-id/flat/CACLU5mSr%2Bt%2B6K_Ww6z1fk_%3DjP%3D5-U0CEqMsHjii8uG_6UWQo-g%40mail.gmail.com
• Commitfest: https://commitfest.postgresql.org/patch/6272/ (note: currently in a closed commitfest, CI red — needs moving to an open CF)

---

🤖 Generated with Claude Code

[NikolayS]

Consolidating everything for this topic into #55 — content moved to #55 (comment)