Re: [DESIGN] Soft DROP TABLE, recoverable drops for PostgreSQL

Hi Amit, Alvaro, Andreas,

Thanks for the comments and suggestions.

Let me first be clearer about the scope of the current patch, since I think
that is the source of some of the concern.
The WIP patch deliberately covers DROP TABLE only. That was meant as the
smallest useful increment to get the mechanism (reserved schema,
OID-derived naming, catalog of dropped objects, restore/purge APIs)
reviewed, not as a claim that all drop paths are protected.
I should have said so explicitly on the thread, and I will make that a
documented limitation rather than leaving it implied.

> AFAICU, the sql_drop event fires after the objects have already been
> removed from the catalogs, and event triggers can only observe the
> command or abort it with an error, not rewrite a DROP into something
> that keeps the relation around. So it would let us reliably record
> what was dropped across all paths (DROP TABLE, DROP SCHEMA ...
> CASCADE, DROP OWNED, cascades), but I don't see how it lets us
> preserve the table for a later restore.

Agreed. I looked at this and it matches what you describe: the object list
is collected in deleteObjectsInList() before the deletion loop, but
EventTriggerSQLDrop() itself does not fire until ProcessUtilitySlow(), by
which point the catalog rows are gone.
Combined with event triggers being observe-or-abort, I do not see a way to
preserve the relation from there.
It would give reliable auditing of what was dropped across all paths, which
is valuable in itself, but not recoverability.
Same conclusion for object_access_hook / OAT_DROP better timing, but that
itself cannot suppress the deletion of a table's indexes, TOAST relation
and owned sequences along with it..

> For actually preserving the data across all the paths, we can actually
> implement it inside the deletion machinery itself, i.e. teaching
> performDeletion to divert a table into a move-to-reserved-schema (when
> the feature is enabled) instead of a hard delete, rather than
> intercepting at the statement layer. That reuses the property that a
> relation in another schema is still fully valid and covers cascades
> for free. This needs much more analysis.
>
> I think this is the right long-term shape, and I would frame it as an
extension of the current design rather than a replacement for it.
The reason the two converge is that the paths in question already share the
same machinery:
DROP TABLE -> RemoveRelations() -> performMultipleDeletions()
DROP SCHEMA/EXTENSION -> RemoveObjects() -> performMultipleDeletions()
DROP OWNED BY -> shdepDropOwned() -> performMultipleDeletions()
and all of them end at deleteOneObject() -> doDeletion() ->
heap_drop_with_catalog() for the relation itself.
So DROP SCHEMA ... CASCADE is not a separate kind of table drop; it reaches
the same place
with the table pulled in by the dependency walk instead of named directly.

A couple of points:
1. Everything above the interception point in my patch i.e the reserved
schema, the naming scheme, the catalog of dropped objects, the restore and
purge APIs, the protections is independent of where the drop is caught, so
moving the interception point later does not change this.
2. The intermediate step is smaller than it looks: at the statement layer
these paths are all DropStmt (OBJECT_SCHEMA, OBJECT_EXTENSION) or
DropOwnedStmt, all visible to the same hook. The extension can enumerate
the affected tables and move them aside before letting the original
statement proceed, which covers the CASCADE forms Zsolt listed without core
changes. The residual gap is the internal and non-statement paths, and that
is where diverting in the deletion machinery is genuinely necessary.

Which is why I would like to suggest keeping this as a contrib extension
for now, and treating the deletion-machinery diversion as the follow-on
once the mechanism has had some real use.
It keeps the footprint small, it is opt-in, and it does not commit core to
a new behaviour for DROP.

> it touches a number of subsystems that evolve with core and would
> need coordinated review: dependency handling and CASCADE (foreign keys,

views, etc.), pg_dump/pg_upgrade treatment of the reserved schema and the
> objects parked in it, event triggers, triggers that reference

other relations, and interaction with logical/DDL replication.
>

This is a fair point. None of these are handled in the current patch:
dependent views and matviews are captured as DDL and dropped rather than
preserved, triggers ride along with the relation but their function bodies
can reference objects that moved, pg_dump/pg_upgrade treatment of the
reserved schema is unaddressed, and a soft-dropped relation is still a live
relation, so it stays in publications, as Zsolt pointed out.
Each of these is extendable within the current structure, and I would
rather work through them incrementally with the scope stated.

Regards,
Surya Poondla