✨ add support for SQLite shorthand foreign key references that implicitly reference primary keys #153

techouse · 2025-11-15T12:17:34Z

This pull request significantly improves the handling of foreign key constraints during SQLite to MySQL migration, especially for shorthand foreign key references that implicitly reference primary keys. The changes add robust logic to resolve referenced columns, handle edge cases where references are ambiguous or mismatched, and introduce targeted unit tests to verify these scenarios.

Enhancements to foreign key handling:

Added _get_table_info and _get_table_primary_key_columns helper methods in transporter.py to fetch table metadata and resolve primary key columns for referenced tables.
Refactored _add_foreign_keys to support shorthand foreign key definitions by resolving missing referenced columns using the referenced table's primary key(s). Added logic to skip and warn on ambiguous or mismatched references, and updated SQL generation to support composite keys.

Testing improvements:

Added two new unit tests:
- test_add_foreign_keys_shorthand_references_primary_key verifies that foreign keys with shorthand references are correctly resolved to the referenced table's primary key.
- test_add_foreign_keys_shorthand_pk_mismatch_is_skipped ensures that foreign keys are skipped and a warning is logged if the column count does not match the referenced table's primary key.

Partially fixes #75
Supersedes #78

coderabbitai · 2025-11-15T12:17:42Z

Walkthrough

The changes introduce two helper methods for SQLite table introspection and enhance foreign key processing to support composite foreign keys. When explicit column references are missing, the system now resolves them by querying the referenced table's primary key columns, enabling correct multi-column FOREIGN KEY clause generation.

Changes

Cohort / File(s)	Summary
SQLite table introspection helpers `src/sqlite3_to_mysql/transporter.py`	Added `_get_table_info()` method to fetch PRAGMA table_xinfo/table_info data; added `_get_table_primary_key_columns()` method to compute visible primary key column names ordered by PK sequence.
Composite foreign key processing `src/sqlite3_to_mysql/transporter.py`	Enhanced `_add_foreign_keys()` to accumulate and sort foreign key rows by sequence; determine referenced columns from explicit REFERENCES or by resolving referenced table's primary key; build multi-column FOREIGN KEY clauses; update constraint naming, logging, and error handling for composite keys.
Unit tests for foreign key resolution `tests/unit/sqlite3_to_mysql_test.py`	Added `test_add_foreign_keys_shorthand_references_primary_key` and `test_add_foreign_keys_shorthand_pk_mismatch_is_skipped` to validate shorthand reference resolution and error handling for unresolved referenced primary key columns.

Sequence Diagram

sequenceDiagram
    participant User
    participant Transporter
    participant SQLite
    participant Logger

    User->>Transporter: _add_foreign_keys(cursor, mysql_conn)
    Transporter->>SQLite: Query foreign keys from table
    SQLite-->>Transporter: Foreign key rows
    
    Note over Transporter: Accumulate rows by id<br/>Sort by sequence
    
    alt References explicitly specify columns
        Transporter->>Transporter: Use referenced columns
    else References omitted (shorthand)
        rect rgb(230, 240, 250)
            Transporter->>Transporter: _get_table_primary_key_columns()
            Transporter->>SQLite: PRAGMA table_info for referenced table
            SQLite-->>Transporter: Table schema (PK columns)
            Transporter->>Transporter: Resolve referenced columns from PK
        end
    end
    
    Note over Transporter: Build multi-column<br/>FOREIGN KEY clause
    Transporter->>SQLite: Execute ALTER TABLE ADD CONSTRAINT
    
    alt Success
        SQLite-->>Transporter: OK
        Transporter->>Logger: Log foreign key added
    else PK resolution failed
        rect rgb(255, 240, 240)
            Transporter->>Logger: Warn unresolved referenced PK
            Transporter->>Transporter: Skip this FK
        end
    end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Primary key column resolution logic and its correctness in determining which columns to reference
Composite foreign key handling—verify accumulation by id, sorting by sequence, and multi-column clause construction
Constraint naming consistency and logging accuracy for composite keys
Test coverage validation: ensure both happy path (shorthand resolution) and error path (unresolved primary key) are adequately exercised

Poem

🐰 With whiskers twitching, PK keys in sight,
Composite foreign bonds, now shining bright,
When references fade, we look ahead,
To primary columns, where links are bred,
No longer lost, our schemas unite! 🎯

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 44.44% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.
Title check	❓ Inconclusive	The title contains a typo ('ass' instead of 'add') and an emoji, reducing clarity. While it attempts to convey support for shorthand REFERENCES, the typo and emoji make it less professional and harder to scan in commit history.	Correct the typo to 'Add support for SQLite shorthand `REFERENCES`' and remove the emoji for a clearer, more professional title suitable for version control history.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description check	✅ Passed	The PR description covers the main enhancements, testing improvements, and linked issues, though it lacks explicit mention of the Type of change, How Has This Been Tested section, and a complete checklist as per the template.
Linked Issues check	✅ Passed	The PR addresses the primary objective from issue #75 by implementing logic to resolve shorthand FOREIGN KEY references using the referenced table's primary key(s) and adding appropriate unit tests.
Out of Scope Changes check	✅ Passed	All changes focus on foreign key constraint handling for shorthand references and primary key resolution, directly aligned with issue #75's objectives for improving SQLite to MySQL migration.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch feat/support-shorthand-fks

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (2)

tests/unit/sqlite3_to_mysql_test.py (2)
1522-1567: Remove unused test parameter.

The mysql_database parameter is created but never used in this test. Consider removing it from the fixture parameters to keep the test focused.
     def test_add_foreign_keys_shorthand_references_primary_key(
         self,
         sqlite_database: str,
-        mysql_database: Engine,
         mysql_credentials: MySQLCredentials,
         mocker: MockFixture,
     ) -> None:
1568-1614: Remove unused test parameter.

The mysql_database parameter is created but never used in this test. Consider removing it from the fixture parameters.
     def test_add_foreign_keys_shorthand_pk_mismatch_is_skipped(
         self,
         sqlite_database: str,
-        mysql_database: Engine,
         mysql_credentials: MySQLCredentials,
         mocker: MockFixture,
     ) -> None:

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5ae7f88 and fa6c27e.

📒 Files selected for processing (2)

src/sqlite3_to_mysql/transporter.py (2 hunks)
tests/unit/sqlite3_to_mysql_test.py (1 hunks)

🧰 Additional context used

📓 Path-based instructions (7)

tests/{unit,func}/**/*.py

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Place unit tests under tests/unit and functional/CLI tests under tests/func

Files:

tests/unit/sqlite3_to_mysql_test.py

tests/**/*.py

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Format test code with Black/isort and respect Flake8’s 88-column soft cap

Files:

tests/unit/sqlite3_to_mysql_test.py

tests/unit/**/*.py

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Add unit tests for new behavior, isolating pure functions

Files:

tests/unit/sqlite3_to_mysql_test.py

src/sqlite3_to_mysql/transporter.py

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

src/sqlite3_to_mysql/transporter.py: Keep SQLite3toMySQL.transfer() flow: create DB (if missing) → create tables → optionally truncate → bulk insert (chunked or streamed) → then create indices → then add foreign keys
On table creation failures due to expression DEFAULTs, retry without DEFAULTs (two-pass _create_table with skip_default=True)
Initialize MySQL capability booleans (e.g., _mysql_fulltext_support, _allow_expr_defaults) early in init, and gate conditional behavior on them
Extend column type translation only in _translate_type_from_sqlite_to_mysql instead of scattering conversions
Handle DEFAULT normalization in _translate_default_for_mysql; return empty string to suppress invalid defaults
During index creation, handle duplicate names by appending numeric suffixes, unless _ignore_duplicate_keys is set to skip retries
Implement chunked transfer with cursor.fetchmany when --chunk is set; otherwise use fetchall with tqdm progress
Only add foreign keys when no table include/exclude filters are set and --without-foreign-keys is not in effect
Implement insert methods: IGNORE (default), UPDATE using ON DUPLICATE KEY UPDATE (with optional VALUES alias), and DEFAULT (no modifiers)
Expose new capability booleans from init of SQLite3toMySQL for downstream logic
Use prepared cursors (cursor(prepared=True)) and batch inserts via executemany for performance
For large transfers, prefer --chunk and preserve commit granularity in any new bulk path

Files:

src/sqlite3_to_mysql/transporter.py

src/sqlite3_to_mysql/**/*.py

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

src/sqlite3_to_mysql/**/*.py: Always wrap dynamic table/index/column names with safe_identifier_length(...) before emitting SQL
Use the class _logger for logging; do not print directly; respect the quiet flag for progress/INFO while always emitting errors

Files:

src/sqlite3_to_mysql/transporter.py

src/**/*.py

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

src/**/*.py: Format with Black (line length 120) and isort (profile=black); adhere to Flake8’s 88-column soft cap to avoid long chained expressions on one line
Preserve and add type hints for new public interfaces

Files:

src/sqlite3_to_mysql/transporter.py

src/sqlite3_to_mysql/{cli.py,transporter.py}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

In debug mode (--debug), surface exceptions instead of swallowing them

Files:

src/sqlite3_to_mysql/transporter.py

🧠 Learnings (14)

📓 Common learnings

Learnt from: CR
Repo: techouse/sqlite3-to-mysql PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-23T18:16:08.709Z
Learning: Applies to src/sqlite3_to_mysql/{mysql_utils.py,sqlite_utils.py} : Add new MySQL/SQLite capability checks in mysql_utils.py/sqlite_utils.py and keep them as dedicated helpers

Learnt from: CR
Repo: techouse/sqlite3-to-mysql PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-23T18:16:08.709Z
Learning: Applies to src/sqlite3_to_mysql/transporter.py : Only add foreign keys when no table include/exclude filters are set and --without-foreign-keys is not in effect

Learnt from: CR
Repo: techouse/sqlite3-to-mysql PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-23T18:16:08.709Z
Learning: Applies to src/sqlite3_to_mysql/transporter.py : Extend column type translation only in _translate_type_from_sqlite_to_mysql instead of scattering conversions

Learnt from: CR
Repo: techouse/sqlite3-to-mysql PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-23T18:16:08.709Z
Learning: Applies to src/sqlite3_to_mysql/transporter.py : Keep SQLite3toMySQL.transfer() flow: create DB (if missing) → create tables → optionally truncate → bulk insert (chunked or streamed) → then create indices → then add foreign keys

Learnt from: CR
Repo: techouse/sqlite3-to-mysql PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-23T18:16:08.709Z
Learning: Applies to src/sqlite3_to_mysql/{mysql_utils.py,sqlite_utils.py} : Centralize dialect/version feature checks, type adaptation, and identifier safety in mysql_utils.py and sqlite_utils.py; add new MySQL capability gates here

Learnt from: CR
Repo: techouse/sqlite3-to-mysql PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-23T18:16:08.709Z
Learning: Applies to src/sqlite3_to_mysql/**/*.py : Always wrap dynamic table/index/column names with safe_identifier_length(...) before emitting SQL

Learnt from: CR
Repo: techouse/sqlite3-to-mysql PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-23T18:16:08.709Z
Learning: Applies to src/sqlite3_to_mysql/cli.py : Make --sqlite-tables and --exclude-sqlite-tables mutually exclusive; setting either implies --without-foreign-keys