Hi Salma
On 28/10/2026 21:17, Salma Elsayed wrote:
> Attached is v11, which removes DROP DATABASE as discussed, applies the
> error style guide to the log messages, and simplifies the TAP test.
Thanks for updating the patch.
A few questions:
== callback functions ==
The comments of RegisterXactCallback and RegisterSubXactCallback suggest
they're meant for extensions: "These functions are intended for use by
dynamically loaded modules.". Are they really the best fit here?
== RegisterDropOrTruncateTable ==
The function already expects reloid, so it could look for rel_name and
rel_namespace itself if needed -- currently the caller needs to do it.
Have you considered simply (relid, is_truncate)?
== doDeletion ==
Is the relname != NULL guard and the "unknown" schemaname really
necessary? I imagined that at this point the relation is locked and must
exist. Also, IMHO silently skipping a log entry or just logging a
placeholder is arguably not a good idea for this feature.
== prepared transactions ==
In DropTableXactCallback there is a whole else if block just to log that
logging the lsn is not possible. Also, I'd argue that the log message in
the following scenario is not very helpful:
psql (20devel)
Type "help" for help.
postgres=# create table t as select 1;
SELECT 1
postgres=# set client_min_messages to log;
SET
postgres=# set log_object_drops to on;
SET
postgres=# begin;
BEGIN
postgres=*# drop table t;
DROP TABLE
postgres=*# prepare transaction 'p';
LOG: table "public.t" (OID 27558) is being dropped inside a prepared
transaction
DETAIL: Automatic recovery LSN capture is not supported for two-phase
commit.
HINT: If this table needs to be recovered, the WAL around the eventual
COMMIT PREPARED will need to be inspected manually.
PREPARE TRANSACTION
postgres=# rollback prepared 'p';
ROLLBACK PREPARED
postgres=# select * from t;
?column?
----------
1
(1 row)
A simplistic approach would be just to make it explicit in the docs and
not log anything.
Thanks for working on this!
Best, Jim
