-
-
Notifications
You must be signed in to change notification settings - Fork 124
transform(update_incoming_fks=True) and --update-incoming-fks #703
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
simonw
wants to merge
11
commits into
main
Choose a base branch
from
claude/investigate-transform-fk-KMTZ7
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Document findings from investigation into how the transform() method handles foreign key constraints that REFERENCE the table being transformed (incoming FKs from other tables). Key findings: - Incoming FKs survive transform because SQLite's ALTER TABLE RENAME automatically updates FK references in the schema - Renaming a referenced COLUMN breaks incoming FKs because SQLite cannot update column references - With FK enforcement ON, transform() correctly detects and rolls back operations that would break FK constraints - With FK enforcement OFF, transforms that break FKs succeed but leave the database in an inconsistent state Also documents a minor issue: leftover temp tables when transform fails.
Additional findings: - Third case that breaks FKs: changing PK away from referenced column (SQLite requires FK targets to be PRIMARY KEY or UNIQUE) - Detection is easy: iterate tables, find incoming FKs, check if referenced columns are being renamed/dropped/losing uniqueness - Automatic fixing for renames is moderate complexity: transform referencing tables first to update their FK constraints - Challenges include circular references and transaction safety - Proposed API: update_incoming_fks=True flag or better error messages
When renaming columns that are referenced by foreign keys in other tables, the transform() method now accepts update_incoming_fks=True to automatically update those FK constraints. Implementation: - Added _get_incoming_fks_needing_update() helper to find tables with FKs pointing to renamed columns - Modified transform() to collect SQL for updating incoming FKs - Execute main transform first (so new column exists), then update incoming FKs, all within the same transaction - Added _skip_fk_validation flag to allow generating SQL for FKs that reference columns that don't exist yet (will exist after main transform completes) Includes test for basic column rename case with FK enforcement ON.
- Fix transform_sql to update other_column for self-referential FKs when the referenced column is being renamed - Skip FK validation during transform when update_incoming_fks=True since self-referential FKs temporarily reference non-existent columns - Add tests for multiple tables referencing renamed column - Add test for self-referential FK handling
When renaming columns that are referenced by foreign keys in other
tables, the --update-incoming-fks flag will automatically update
those FK constraints.
Example:
sqlite-utils transform mydb.db authors \
--rename id author_pk \
--update-incoming-fks
This will rename the 'id' column to 'author_pk' and also update any
foreign key constraints in other tables (e.g., books.author_id)
that reference the renamed column.
Document the new update_incoming_fks parameter in: - Python API docs (python-api.rst): New section explaining usage - CLI docs (cli.rst): Document --update-incoming-fks flag
- Add type annotation for incoming_fk_sqls: List[str] - Use self.db.table() instead of self.db[] to get Table type - Use setattr/getattr for _skip_fk_validation to avoid attr-defined error
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #703 +/- ##
==========================================
+ Coverage 95.15% 95.17% +0.02%
==========================================
Files 8 8
Lines 3114 3153 +39
==========================================
+ Hits 2963 3001 +38
- Misses 151 152 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Black 26.1.0 introduced new formatting rules that affect this codebase. Pin to >=26.1.0 to ensure consistent formatting across all environments. Black 26.1.0 requires Python >=3.10 which matches this project's minimum.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Add
update_incoming_fksparameter totransform()that automatically updates foreign key constraints in other tables when renaming columns that are referenced by those FKs.Problem
When renaming a column that is referenced by foreign keys in other tables, the transform fails with a "foreign key mismatch" error (with FK enforcement on) or silently breaks the FK constraints (with FK enforcement off). Users had to manually transform each referencing table first to update their FK constraints before transforming the main table.
Solution
New
update_incoming_fks=Trueparameter fortransform()and--update-incoming-fksCLI flag that:employees.manager_id -> employees.id)Example
Investigation notes: https://github.com/simonw/sqlite-utils/blob/c727ceab0763ff02fe10da720f427db5e9693e4e/TRANSFORM_FK_INVESTIGATION.md