Ujjwaljain16 commented on issue #38629: URL: https://github.com/apache/superset/issues/38629#issuecomment-5992756721
Hi all, I'd like to pick this up. Here's what I found first, so we don't repeat earlier work. **Previous attempt** #41994 tried to fix this with `ON DELETE` rules on the `ab_user` foreign keys. @michael-s-molina asked for a SIP before any code, since tables like `logs` and `saved_query` can't all simply be set to NULL. The author narrowed the scope, but @rusackas still wanted sign-off on the SET NULL vs CASCADE semantics. The migration also had a `down_revision` clash. The author closed it on 2026-08-28 and said they'd revisit once a SIP existed. They also noted that a DB-level cascade on `favstar` bypasses `FavStarUpdater.after_delete`, so tagging data would leak. **Not MariaDB-specific** It also reproduces on PostgreSQL. @gdudau and @hannespr (6.1.0) both hit `key_value_created_by_fk_fkey` violations when deleting a user. **Still present on master (`7a2913e2db`)** These `ab_user` foreign keys declare no `ondelete`: - `logs.user_id` - `favstar.user_id` - `key_value.created_by_fk` / `changed_by_fk` - `query.user_id` - `saved_query.user_id` - `tab_state.user_id` - `user_attribute.user_id` - `user_favorite_tag.user_id` - `slices.last_saved_by_fk` - `created_by_fk` / `changed_by_fk` from `AuditMixinNullable`, which is used by many models Newer tables already declare `ondelete="CASCADE"` on `ab_user.id`: `database_user_oauth2_tokens`, `task_subscribers`, the extension storage table and `subjects`. So the codebase is already moving in that direction for user-scoped data, but the older tables above were never covered. **Existing SIP/discussion** I couldn't find a SIP or discussion that decides this. #39464 (SIP-208) is soft delete for charts, dashboards and datasets, which is a different topic. The problem has come up repeatedly: #8752, #13345, #29512 and discussion #40137. **Proposal** I'd like to write a SIP that decides the policy per category of data: - audit data: keep the row, clear the user - personal state: delete with the user - authored or owned content: reassign to a chosen user, or refuse with a clear error instead of an IntegrityError It would also compare doing this at the DB level with doing it in the application. Superset already has `UserDAO.delete`, which does custom cleanup (`_delete_subject`) before delegating to the base delete. That might be a natural place for the application-level option and would avoid a migration across all these constraints. @michael-s-molina @rusackas does this direction sound right? If someone is already working on it, tell me and I'll help there instead. Otherwise I'll start a `[DISCUSS]` thread on dev@ once I have a draft. -- 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]
