Hello, I want to come back to the DROP_CASCADE in drop_sub_conflict_log_table(). This was discussed in June, but I think the follow-up items from that discussion got lost before the commit.
What I found in the thread: * Shveta noticed that DROP SUBSCRIPTION also drops tables inheriting from the CLT, because the CLT is removed with DROP_CASCADE [1]. * Dilip's position was that this kind of indirect usage is the user's problem, as long as it is documented [2], and Shveta agreed, provided that the docs get a CAUTION saying DROP SUBSCRIPTION cascades to the CLT "and all its dependent objects, including any user-created inherited tables, view etc" [3]. * Later Amit also said that it is fine to block views on the CLT for the first version [4]. What is in master now: inheritance is blocked, views are not, and create_subscription.sgml only says that the table is dropped, there isn't anything about other objects. drop_subscription.sgml also still says that CASCADE/RESTRICT "do not have any effect, since there are no dependencies on subscriptions", which is no longer true, DROP SUBSCRIPTION ... RESTRICT cascades. The June discussion only mentioned inherited tables and views, while the cascade goes through everything that depends on the CLT or on its row type. I/Claude tried a bunch of object types, I think the realistic ones are: * views and materialized views on the CLT, and views built on top of those * SQL functions referencing the CLT in a BEGIN ATOMIC body, or taking/returning its row type (and with them, functions calling these) * maybe table columns typed as the CLT row type, if someone archives conflict rows that way All of these are silently dropped (there is a NOTICE with the list, but no error) by DROP SUBSCRIPTION, and also by ALTER SUBSCRIPTION ... SET (conflict_log_destination = log). The ALTER case seems more surprising, as it looks like a logging setting change. Constraints, indexes, triggers or policies on other tables are also dropped if their expressions reference the row type, but I don't think anybody would use those in practice. Two more things I want to point out, which I think wasn't mentioned before: 1. The dropped objects don't have to belong to the subscription owner, and the owner doesn't need any privilege on them. This is how I originally found the issue, as my claude feature crosscheck classified this as a security issue. DROP ... CASCADE crosses ownership boundaries too, but there somebody explicitly asks for it. In this case nobody does (rather, it happens even with DROP SUBSCRIPTION RESTRICT), and the person creating the monitoring views and the subscription owner can easily be different people. 2. Because the deletion uses PERFORM_DELETION_INTERNAL, sql_drop event triggers don't see the cascaded objects either, pg_event_trigger_dropped_objects() only reports the subscription. Options I see: a. Do what was mentioned in June: document the cascade properly in create/alter/drop_subscription, and fix the CASCADE/RESTRICT paragraph. b. Make DROP SUBSCRIPTION honor its existing CASCADE | RESTRICT: with the default RESTRICT, error out if anything other than the CLT's own objects depends on the CLT, the same way DROP TABLE does. ALTER SUBSCRIPTION has no CASCADE, so there it could error out with a hint to drop the dependent objects first. (b would need some documentation changes too) c. Block views/functions/... on the CLT, as Amit suggested, but that seems hard to do completely, there are many ways to reference a table or its row type. So a or b seem more realistic to me, and b seems relatively easy to implement to me: one additional arg to drop_sub_conflict_log_table passed to performDeletion instead of a fixed DROP_CASCADE, and calling it earlier in DropSubscription, and everything seems to work properly in my local testing. It would also match how other DROP commands behave, and it doesn't break the monitoring use case which was a reason to allow views outside pg_conflict [5]. [1]: https://postgr.es/m/cajpy0ubhjgqd+oya9uk8bhyk61xwhef3le1qxkotwolcqcq...@mail.gmail.com [2]: https://postgr.es/m/CAFiTN-teHjcn7OkfE7n-e=skgogeejcz+4hcfb2fad1-55z...@mail.gmail.com [3]: https://postgr.es/m/cajpy0udane7j9hloy0jnwjqxtuaerhiykp5ba9hjskypk0s...@mail.gmail.com [4]: https://postgr.es/m/CAA4eK1JpOUo=3hwcywkwyg8ujv+wcxbw3y5sqphgp1rutt2...@mail.gmail.com [5]: https://postgr.es/m/CAJpy0uDoa0CYWkxj52h=RM53acfsqjRihCfKrm8W=vrvhg0...@mail.gmail.com
