From 49abc4ea237885a456d67e3045a8732f23d048cf Mon Sep 17 00:00:00 2001 From: Zsolt Parragi Date: Fri, 16 Oct 2026 21:32:54 +0000 Subject: [PATCH v1] Honor RESTRICT when dropping a subscription's conflict log table. Previously, the conflict log table was dropped with DROP_CASCADE, both by DROP SUBSCRIPTION and by ALTER SUBSCRIPTION ... SET (conflict_log_destination = 'log'). So any user objects that depend on the table, such as views or functions referencing it or its row type, were silently dropped as well. These objects need not be owned by the subscription owner, and the ALTER case looks like a mere logging setting change to the user. This commit makes DROP SUBSCRIPTION pass its CASCADE | RESTRICT behavior down to the deletion of the conflict log table, and ALTER SUBSCRIPTION always uses RESTRICT, so both fail if other objects depend on the table, as DROP TABLE does. In DROP SUBSCRIPTION, the table is now dropped before the non-transactional cleanup of the replication slot and origin, so that an error leaves the subscription intact. The docs claimed that CASCADE and RESTRICT have no effect for DROP SUBSCRIPTION, which was no longer true after a5918fddf1. Update them accordingly. Oversight in a5918fddf1. Author: Zsolt Parragi Discussion: https://postgr.es/m/CAFiTN-u5D5o_AGNbHRZHaOqAMWkxLf%2BhSk_r9X3gv6HbLOB5%2Bg%40mail.gmail.com --- doc/src/sgml/ref/alter_subscription.sgml | 1 + doc/src/sgml/ref/drop_subscription.sgml | 15 +++++++++--- src/backend/commands/subscriptioncmds.c | 28 ++++++++++++++-------- src/test/regress/expected/subscription.out | 18 +++++++++++++- src/test/regress/sql/subscription.sql | 11 ++++++++- 5 files changed, 58 insertions(+), 15 deletions(-) diff --git a/doc/src/sgml/ref/alter_subscription.sgml b/doc/src/sgml/ref/alter_subscription.sgml index 545264e8a0a..3ac77b9c7c4 100644 --- a/doc/src/sgml/ref/alter_subscription.sgml +++ b/doc/src/sgml/ref/alter_subscription.sgml @@ -367,6 +367,7 @@ ALTER SUBSCRIPTION name RENAME TO < parameter is set to table or all, the system automatically creates the conflict log table. Conversely, if the destination is changed to log, the conflict log table is automatically dropped. + This fails if any other objects depend on the conflict log table. diff --git a/doc/src/sgml/ref/drop_subscription.sgml b/doc/src/sgml/ref/drop_subscription.sgml index e4c646fcf2a..42eb67bd5a0 100644 --- a/doc/src/sgml/ref/drop_subscription.sgml +++ b/doc/src/sgml/ref/drop_subscription.sgml @@ -70,12 +70,21 @@ DROP SUBSCRIPTION [ IF EXISTS ] name CASCADE - RESTRICT + + + Automatically drop objects that depend on the subscription's conflict + log table (such as views), and in turn all objects that depend on those + objects (see ). + + + + + RESTRICT - These key words do not have any effect, since there are no dependencies - on subscriptions. + Refuse to drop the subscription if any objects depend on its conflict + log table. This is the default. diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index 36bb20a1f6e..3dad412423d 100644 --- a/src/backend/commands/subscriptioncmds.c +++ b/src/backend/commands/subscriptioncmds.c @@ -150,7 +150,8 @@ static bool alter_sub_conflict_log_dest(Subscription *sub, ConflictLogDest newlogdest, Oid *conflicttablerelid); static void drop_sub_conflict_log_table(Oid subid, char *subname, - Oid subconflictlogrelid); + Oid subconflictlogrelid, + DropBehavior behavior); /* * Common option parsing function for CREATE and ALTER SUBSCRIPTION commands. @@ -1620,7 +1621,7 @@ alter_sub_conflict_log_dest(Subscription *sub, ConflictLogDest oldlogdest, if (!want_table) { drop_sub_conflict_log_table(sub->oid, sub->name, - sub->conflictlogrelid); + sub->conflictlogrelid, DROP_RESTRICT); update_relid = true; } } @@ -2559,9 +2560,10 @@ construct_subserver_conninfo(Oid subserver, Oid subowner, char **err) * Drop subscription's conflict log table * * The conflict log table is registered as an internal dependency of the - * subscription. This function removes the dependency by performing a - * cascading deletion on the subscription object, which in turn drops the - * associated conflict log table. + * subscription. This function removes the dependency by deleting the + * dependents of the subscription object, which in turn drops the associated + * conflict log table. With DROP_RESTRICT, this fails if other objects + * depend on the conflict log table. * * This is used to clean up conflict log tables that are no longer required, * preventing accumulation of stale or orphaned relations. @@ -2571,7 +2573,8 @@ construct_subserver_conninfo(Oid subserver, Oid subowner, char **err) * mechanism. */ static void -drop_sub_conflict_log_table(Oid subid, char *subname, Oid subconflictlogrelid) +drop_sub_conflict_log_table(Oid subid, char *subname, Oid subconflictlogrelid, + DropBehavior behavior) { /* Drop any dependent conflict log table */ if (OidIsValid(subconflictlogrelid)) @@ -2589,7 +2592,7 @@ drop_sub_conflict_log_table(Oid subid, char *subname, Oid subconflictlogrelid) * conflict log table is deleted while the subscription remains. */ ObjectAddressSet(object, SubscriptionRelationId, subid); - performDeletion(&object, DROP_CASCADE, + performDeletion(&object, behavior, PERFORM_DELETION_INTERNAL | PERFORM_DELETION_SKIP_ORIGINAL); @@ -2709,6 +2712,14 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel) if (slotname) PreventInTransactionBlock(isTopLevel, "DROP SUBSCRIPTION"); + /* + * Drop the conflict log table first. With RESTRICT this fails if other + * objects depend on it, so do it before any non-transactional cleanup + * below. + */ + drop_sub_conflict_log_table(subid, subname, subconflictlogrelid, + stmt->behavior); + ObjectAddressSet(myself, SubscriptionRelationId, subid); EventTriggerSQLDropAddObject(&myself, true, true); @@ -2783,9 +2794,6 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel) replorigin_drop_by_name(originname, true, false); } - /* Drop subscription's conflict log table */ - drop_sub_conflict_log_table(subid, subname, subconflictlogrelid); - /* Clean up dependencies */ deleteDependencyRecordsFor(SubscriptionRelationId, subid, false); deleteSharedDependencyRecordsFor(SubscriptionRelationId, subid, 0); diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out index 681dc66dca6..cb66ee1b241 100644 --- a/src/test/regress/expected/subscription.out +++ b/src/test/regress/expected/subscription.out @@ -834,10 +834,20 @@ ERROR: 42501 -- CLEANUP: DROP SUBSCRIPTION reaps the table ALTER SUBSCRIPTION regress_conflict_test1 DISABLE; ALTER SUBSCRIPTION regress_conflict_test1 SET (slot_name = NONE); +-- fail - objects depending on the conflict log table need CASCADE +CREATE VIEW regress_clt_view AS SELECT relid FROM :clt1; +\set VERBOSITY sqlstate +ALTER SUBSCRIPTION regress_conflict_test1 SET (conflict_log_destination = 'log'); +ERROR: 2BP01 +DROP SUBSCRIPTION regress_conflict_test1; +ERROR: 2BP01 +DROP SUBSCRIPTION regress_conflict_test1 RESTRICT; +ERROR: 2BP01 +\set VERBOSITY default -- Verify the table OID for reap check SELECT 'pg_conflict.pg_conflict_log_' || oid AS internal_tablename FROM pg_subscription WHERE subname = 'regress_conflict_test1' \gset SET client_min_messages = WARNING; -DROP SUBSCRIPTION regress_conflict_test1; +DROP SUBSCRIPTION regress_conflict_test1 CASCADE; -- should return NULL, meaning the conflict log table was reaped via dependency SELECT to_regclass(:'internal_tablename'); to_regclass @@ -845,6 +855,12 @@ SELECT to_regclass(:'internal_tablename'); (1 row) +SELECT to_regclass('regress_clt_view'); + to_regclass +------------- + +(1 row) + -- -- Additional Namespace and Table Protection Tests -- diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql index cfeebaf9302..bc0afba2793 100644 --- a/src/test/regress/sql/subscription.sql +++ b/src/test/regress/sql/subscription.sql @@ -636,14 +636,23 @@ DROP TABLE :clt1; ALTER SUBSCRIPTION regress_conflict_test1 DISABLE; ALTER SUBSCRIPTION regress_conflict_test1 SET (slot_name = NONE); +-- fail - objects depending on the conflict log table need CASCADE +CREATE VIEW regress_clt_view AS SELECT relid FROM :clt1; +\set VERBOSITY sqlstate +ALTER SUBSCRIPTION regress_conflict_test1 SET (conflict_log_destination = 'log'); +DROP SUBSCRIPTION regress_conflict_test1; +DROP SUBSCRIPTION regress_conflict_test1 RESTRICT; +\set VERBOSITY default + -- Verify the table OID for reap check SELECT 'pg_conflict.pg_conflict_log_' || oid AS internal_tablename FROM pg_subscription WHERE subname = 'regress_conflict_test1' \gset SET client_min_messages = WARNING; -DROP SUBSCRIPTION regress_conflict_test1; +DROP SUBSCRIPTION regress_conflict_test1 CASCADE; -- should return NULL, meaning the conflict log table was reaped via dependency SELECT to_regclass(:'internal_tablename'); +SELECT to_regclass('regress_clt_view'); -- -- Additional Namespace and Table Protection Tests -- 2.56.0