Design review of the log_object_drops / restore-LSN-on-DROP feature, produced live during the Hacking Postgres session of 2026-07-15 (Kirk Wolak, Andrey Borodin, Nikolay Samokhvalov).
Two agents ran, answering two different framings of Andrey's question: "can we spend the creativity budget on generalizing the feature — why is it needed, why is it valuable, how can this be done more in a Postgres way?"
All code claims below were checked against master as of 2026-07-15. The correctness bug they surfaced is filed separately as #53 — read that one first, it is the load-bearing finding.
Memo 1 — the general question (extension allowed)
TL;DR: Everything this patch needs already exists in core: OAT_DROP fires for tables and databases, XactLastCommitEnd is an exported global set before XACT_EVENT_COMMIT callbacks run, and XLogRestorePoint() is an exported function. The feature could be an extension emitting a named restore point pre-commit plus a log line announcing it — and the only core-worthy piece is documenting/stabilizing the commit-LSN-in-callback contract.
Verified facts
| Claim |
Location |
OAT_DROP fires for tables |
dependency.c:1320 |
OAT_DROP fires for databases (event triggers do not cover DROP DATABASE) |
dbcommands.c:1741 |
XactLastCommitEnd is PGDLLIMPORT |
xlog.h:35 |
...assigned in RecordTransactionCommit before CallXactCallbacks(XACT_EVENT_COMMIT) |
xact.c:1602 vs xact.c:2449 |
| ...but skipped when a xact writes no WAL (stale-value trap) |
xact.c:1603 |
XLogRestorePoint() exported |
xlog.h:274 |
RECOVERY_TARGET_NAME stops after the first matching restore point |
xlogrecovery.c:2724-2741 |
XACT_EVENT_PRE_COMMIT is outside the commit critical section |
xact.c:2322 |
Key points
Kirill Reshke's unanswered question. In March 2025 he asked on the thread: "What stops us from logging all the same inside object access hook defined by extension?" Nobody replied. The answer is: nothing. An extension can hook OAT_DROP, stash names, track subxact aborts via RegisterSubXactCallback, write XLogRestorePoint() in PRE_COMMIT, and log XactLastCommitEnd at COMMIT — roughly 200 lines, zero core changes. This objection will be raised again by a committer; the thread needs an answer ready.
Restore points vs. log lines — not either/or. A log line lives on the machine you may have just destroyed; a restore point is a WAL record — archived, replicated, readable by pg_waldump with zero logs. recovery_target_name also has no inclusive/exclusive footgun. Because RECOVERY_TARGET_NAME stops after the first match, the restore point must be written at PRE_COMMIT, and names need an LSN/xid uniquifier first (a twice-dropped-and-recreated table must not send you to the wrong epoch). Recommendation: restore point as the durable artifact, log line as the discovery channel.
Orthogonality. The feature is a composition of three facilities that already exist (drop-event surface, commit-LSN exposure, recovery landmarks). That composition is the Postgres way — and is the argument for it living outside core. A general "record landmark on DDL class" core mechanism is architecture-astronomy.
Docs. Primary home is the Continuous Archiving / PITR chapter, cross-linked from the DROP TABLE reference page — the page a panicking DBA actually opens. Burying it in runtime-config-logging hides it from the only person who needs it.
Memo 2 — the same question with the extension option removed
Per Andrey on the call: "maybe we can ask it to consider again if we don't want anything like an extension", and Kirk: "we try to get away with it in the core, and only push it into a contrib if we're forced to by hackers."
TL;DR: In-core changes the answer — drop the restore-point machinery entirely. Core can hard-wire an emit call into CommitTransaction and log a copy-pasteable PITR recipe with the exact commit-record LSN, which no extension can observe. And kill v5's XactCallback signature change — that, not performance, is what committers will reject.
Why in-core changes the shape
The extension needed a PRE_COMMIT restore point because it couldn't trust the commit LSN (the XactLastCommitEnd stale-value trap). Core reads the LSN at the source — v5 already threads it out of RecordTransactionCommit. With an exact, correctly-chosen LSN, a restore point becomes pure overhead: extra WAL per DROP, name-collision semantics, and a second mechanism to document. The commit record is already in the archive; the log line is its index. Core also owns the recovery_target_* docs, so it can print the exact recipe and guarantee it stays correct.
Kirk's "it's not heavy" argument — steelmanned, then attacked
When off, the cost is literally one bool test in doDeletion (dependency.c:1613) and one in dropdb (dbcommands.c:1874) — per-drop, not per-tuple, not in any hot path. He is right that performance is a non-issue. But committers won't argue performance. They'll argue:
- The ABI break. v5 changes the
RegisterXactCallback function-pointer signature to pass commit_lsn. That breaks pgaudit, sepgsql, and every extension with a xact callback — indefensible when core code can just call a function directly. This is the single most likely rejection trigger and it is trivially avoidable.
- Log-format-as-API and scope creep ("why not TRUNCATE?"). Answer: DROP is uniquely non-recoverable-in-place; draw that line explicitly in the docs.
- Reshke's "why not an extension" — needs the answer below.
What actually gets it committed: remove the callback ABI change, keep the diff under ~300 lines, and make the log line demonstrably correct (see #53 — a committer who tests the current recipe and replays the drop anyway will bounce the patch instantly).
The strongest in-core design
- GUC
log_object_drops, bool, PGC_SUSET, default off (as v5).
- Registration where v5 puts it:
doDeletion for RELKIND_RELATION/PARTITIONED_TABLE, non-temp, plus dropdb.
- Emission: a direct
AtEOXact_LoggedDrops(commit_start_lsn) call in CommitTransaction right after RecordTransactionCommit, with abort-side cleanup next to the other AtEOXact_* calls. No callback registration, no ABI change.
- Output, per object:
LOG: DROP TABLE: "s.t" (OID n), xid X, commit record at LSN A/B plus an errhint() giving the exact recovery recipe.
- Against "the log is on the machine you just destroyed": DROP destroys data, not the server or its (typically shipped) logs; and the LSN merely locates a commit record that already lives in the archive — worst case
pg_waldump re-derives it.
The pitch to hackers
When a table is dropped in production, the DBA's recovery time is dominated by one question: what LSN do I recover to? Today, answering it means pg_waldump archaeology under incident pressure. This patch answers it in one grep, using information only the commit path has — the commit record's LSN before it's discarded. No event trigger can do this: they don't fire for DROP DATABASE and can't see the commit LSN. It belongs in core exactly like log_checkpoints: operational breadcrumbs for the moment you need them most. Off by default, one branch in the drop path, zero cost until a DBA turns it on — and the first time it's used, it pays for itself in hours of downtime.
Where the two memos disagree
Memo 1 says ship it as an extension/contrib; Memo 2, told to assume core, says core genuinely buys something an extension cannot: the exact commit-record LSN, and ownership of the docs that make the recipe correct. These are consistent — the disagreement is entirely about whether that gain justifies core surface area, which is a judgement call for -hackers, not for an agent. What both agree on:
Refs
🤖 Generated with Claude Code
Design review of the
log_object_drops/ restore-LSN-on-DROP feature, produced live during the Hacking Postgres session of 2026-07-15 (Kirk Wolak, Andrey Borodin, Nikolay Samokhvalov).Two agents ran, answering two different framings of Andrey's question: "can we spend the creativity budget on generalizing the feature — why is it needed, why is it valuable, how can this be done more in a Postgres way?"
All code claims below were checked against
masteras of 2026-07-15. The correctness bug they surfaced is filed separately as #53 — read that one first, it is the load-bearing finding.Memo 1 — the general question (extension allowed)
TL;DR: Everything this patch needs already exists in core:
OAT_DROPfires for tables and databases,XactLastCommitEndis an exported global set beforeXACT_EVENT_COMMITcallbacks run, andXLogRestorePoint()is an exported function. The feature could be an extension emitting a named restore point pre-commit plus a log line announcing it — and the only core-worthy piece is documenting/stabilizing the commit-LSN-in-callback contract.Verified facts
OAT_DROPfires for tablesdependency.c:1320OAT_DROPfires for databases (event triggers do not cover DROP DATABASE)dbcommands.c:1741XactLastCommitEndisPGDLLIMPORTxlog.h:35RecordTransactionCommitbeforeCallXactCallbacks(XACT_EVENT_COMMIT)xact.c:1602vsxact.c:2449xact.c:1603XLogRestorePoint()exportedxlog.h:274RECOVERY_TARGET_NAMEstops after the first matching restore pointxlogrecovery.c:2724-2741XACT_EVENT_PRE_COMMITis outside the commit critical sectionxact.c:2322Key points
Kirill Reshke's unanswered question. In March 2025 he asked on the thread: "What stops us from logging all the same inside object access hook defined by extension?" Nobody replied. The answer is: nothing. An extension can hook
OAT_DROP, stash names, track subxact aborts viaRegisterSubXactCallback, writeXLogRestorePoint()in PRE_COMMIT, and logXactLastCommitEndat COMMIT — roughly 200 lines, zero core changes. This objection will be raised again by a committer; the thread needs an answer ready.Restore points vs. log lines — not either/or. A log line lives on the machine you may have just destroyed; a restore point is a WAL record — archived, replicated, readable by
pg_waldumpwith zero logs.recovery_target_namealso has no inclusive/exclusive footgun. BecauseRECOVERY_TARGET_NAMEstops after the first match, the restore point must be written at PRE_COMMIT, and names need an LSN/xid uniquifier first (a twice-dropped-and-recreated table must not send you to the wrong epoch). Recommendation: restore point as the durable artifact, log line as the discovery channel.Orthogonality. The feature is a composition of three facilities that already exist (drop-event surface, commit-LSN exposure, recovery landmarks). That composition is the Postgres way — and is the argument for it living outside core. A general "record landmark on DDL class" core mechanism is architecture-astronomy.
Docs. Primary home is the Continuous Archiving / PITR chapter, cross-linked from the
DROP TABLEreference page — the page a panicking DBA actually opens. Burying it in runtime-config-logging hides it from the only person who needs it.Memo 2 — the same question with the extension option removed
Per Andrey on the call: "maybe we can ask it to consider again if we don't want anything like an extension", and Kirk: "we try to get away with it in the core, and only push it into a contrib if we're forced to by hackers."
TL;DR: In-core changes the answer — drop the restore-point machinery entirely. Core can hard-wire an emit call into
CommitTransactionand log a copy-pasteable PITR recipe with the exact commit-record LSN, which no extension can observe. And kill v5'sXactCallbacksignature change — that, not performance, is what committers will reject.Why in-core changes the shape
The extension needed a PRE_COMMIT restore point because it couldn't trust the commit LSN (the
XactLastCommitEndstale-value trap). Core reads the LSN at the source — v5 already threads it out ofRecordTransactionCommit. With an exact, correctly-chosen LSN, a restore point becomes pure overhead: extra WAL per DROP, name-collision semantics, and a second mechanism to document. The commit record is already in the archive; the log line is its index. Core also owns therecovery_target_*docs, so it can print the exact recipe and guarantee it stays correct.Kirk's "it's not heavy" argument — steelmanned, then attacked
When off, the cost is literally one bool test in
doDeletion(dependency.c:1613) and one indropdb(dbcommands.c:1874) — per-drop, not per-tuple, not in any hot path. He is right that performance is a non-issue. But committers won't argue performance. They'll argue:RegisterXactCallbackfunction-pointer signature to passcommit_lsn. That breaks pgaudit, sepgsql, and every extension with a xact callback — indefensible when core code can just call a function directly. This is the single most likely rejection trigger and it is trivially avoidable.What actually gets it committed: remove the callback ABI change, keep the diff under ~300 lines, and make the log line demonstrably correct (see #53 — a committer who tests the current recipe and replays the drop anyway will bounce the patch instantly).
The strongest in-core design
log_object_drops, bool,PGC_SUSET, default off (as v5).doDeletionforRELKIND_RELATION/PARTITIONED_TABLE, non-temp, plusdropdb.AtEOXact_LoggedDrops(commit_start_lsn)call inCommitTransactionright afterRecordTransactionCommit, with abort-side cleanup next to the otherAtEOXact_*calls. No callback registration, no ABI change.LOG: DROP TABLE: "s.t" (OID n), xid X, commit record at LSN A/Bplus anerrhint()giving the exact recovery recipe.pg_waldumpre-derives it.The pitch to hackers
Where the two memos disagree
Memo 1 says ship it as an extension/contrib; Memo 2, told to assume core, says core genuinely buys something an extension cannot: the exact commit-record LSN, and ownership of the docs that make the recipe correct. These are consistent — the disagreement is entirely about whether that gain justifies core surface area, which is a judgement call for -hackers, not for an agent. What both agree on:
XactCallbackABI change should go.Refs
🤖 Generated with Claude Code