mikebridge commented on PR #39859:
URL: https://github.com/apache/superset/pull/39859#issuecomment-4502344978
> ... A few things worth addressing before merge:
@aminghadersohi (via claude):
Thanks for the thorough review and the kind words on the runbook.
All three points addressed in `0ebf2ab68e`:
1. `fk["name"]` guard: agreed on the asymmetry — symmetrized via if not
`fk.get("name")`: continue at the top of the re-add loop. MySQL/InnoDB always
names FKs so this is defensive, but the consistency is worth it.
2. ondelete whitelist: added a `_VALID_ONDELETE_ACTIONS` frozenset
(`{CASCADE, SET NULL, RESTRICT, NO ACTION}`) and a `RuntimeError` on unexpected
reflected values. Excludes `SET DEFAULT` since InnoDB silently downgrades it
to `NO ACTION`.
3. recreate="always" on Postgres: great catch. Refactored to a three-way
dialect split for the two UNIQUE-bearing tables:
- **Postgres**: reflect UNIQUE by name → drop by name → direct ALTER.
Avoids the CREATE TABLE AS SELECT → DROP → RENAME and its ACCESS EXCLUSIVE lock
window entirely; only the PK index build remains locked.
- **MySQL**: keeps `recreate="always"` (with the FK drop dance) because
InnoDB binds `FK`s to the `UNIQUE`'s underlying index — dropping the `UNIQUE`
directly raises **ERROR 1553** ("needed in a foreign key constraint"). Worth
knowing about — I tried the direct path first to be consistent across backends,
and that's the reason for the asymmetric handling.
- **SQLite**: keeps `recreate="always"` because unnamed `UNIQUE`s
reflect with `name=None`.
_Local verification_: downgrade-then-upgrade against MySQL with ~12M total
seeded junction rows runs in 1m 41s, matching the prior baseline.
--
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]
