mikebridge commented on PR #41549: URL: https://github.com/apache/superset/pull/41549#issuecomment-4971338491
Thanks — agreed that the hand-maintained cascade is the main long-term maintenance risk here. The explicit deletion sequence is intentional for this PR. We need the purge to behave consistently when SQLite FK enforcement is off, avoid the ORM loading and traversing large relationship graphs, preserve ordinary deletion blockers, and distinguish owned children / association rows from independently owned referrers. The versioning side also cannot be represented as a simple ORM cascade: Continuum transactions may be shared across entities, so only the target entity's shadow/change rows are removed and a transaction is deleted only after it becomes orphaned. Using Core deletes also means the relevant tag and dataset-PVM listener cleanup has to be reproduced explicitly. The savepoint around the sequence lets all of that roll back cleanly if the eligibility recheck loses a restore race or a restrictive FK blocks the parent delete. I agree that this creates a future-update obligation whenever a chart/dashboard/dataset gains another owned child, M:N association, or delete listener. A useful follow-up direction would be to replace the scattered type branches with a declarative per-entity purge registry describing: - ordinary deletion validators; - owned child and association tables; - listener-equivalent cleanup hooks; - version-shadow ownership predicates; and - relationships that are intentionally preserved or treated as blockers. We could pair that registry with contract tests that introspect mapped relationships/FKs and fail when a new owned or cascading relationship is added without an explicit purge policy. That would turn this from reviewer knowledge into a CI tripwire while preserving the cross-database and race-safety properties required here. Direct CLI coverage would also be a good small follow-up. _Response prepared via Codex after checking the implementation and deletion-retention spec._ -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
