-
-
Couldn't load subscription status.
- Fork 35
🦺 improve robustness and correctness of MySQL-to-SQLite migrations #111
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
Conversation
…expressions in MySQL to SQLite translation
…ta types and fallback scenarios
…QLite translation
…glot transpilation
WalkthroughAdds sqlglot-based transpilation and robust identifier quoting/escaping to the MySQL→SQLite transporter; expands type and default translation rules with sqlglot fallbacks; updates CREATE TABLE / index / foreign-key DDL generation; and adds/updates unit tests and a pylint disable tweak. Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(248,249,250,0.9)
participant Caller
participant translate_type as _translate_type_from_mysql_to_sqlite
participant BuiltIn as Built-in Mapping
participant transpile_type as _transpile_mysql_type_to_sqlite
participant sqlglot
end
Caller->>translate_type: provide MySQL type string
translate_type->>BuiltIn: check known mappings
alt built-in mapping found
BuiltIn-->>translate_type: SQLite type
else fallback to transpile
translate_type->>transpile_type: request sqlglot transpilation
transpile_type->>sqlglot: parse/derive SQLite-friendly type
sqlglot-->>transpile_type: derived type or None
transpile_type-->>translate_type: inferred SQLite type (or None)
end
translate_type-->>Caller: SQLite type (or default TEXT)
sequenceDiagram
rect rgba(248,249,250,0.9)
participant Caller
participant translate_def as _translate_default_from_mysql_to_sqlite
participant transpile_expr as _transpile_mysql_expr_to_sqlite
participant sqlglot
participant Escaper as Legacy Escaper
end
Caller->>translate_def: provide MySQL default expression & context
translate_def->>transpile_expr: attempt sqlglot-based transpile
alt transpilation success
transpile_expr->>sqlglot: parse/convert expression
sqlglot-->>transpile_expr: SQLite expression
transpile_expr-->>translate_def: converted expression
else transpilation fail
transpile_expr-->>translate_def: None
translate_def->>Escaper: apply legacy escaping/quoting logic
Escaper-->>translate_def: escaped default string
end
translate_def-->>Caller: DEFAULT clause (SQLite-ready)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (4)src/mysql_to_sqlite3/transporter.py📄 CodeRabbit inference engine (.github/copilot-instructions.md)
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #111 +/- ##
==========================================
- Coverage 94.44% 93.44% -1.00%
==========================================
Files 8 8
Lines 774 900 +126
==========================================
+ Hits 731 841 +110
- Misses 43 59 +16 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/mysql_to_sqlite3/transporter.py (1)
297-299: Bug: string membership used for TIMESTAMP check.
if data_type in "TIMESTAMP":is a fragile substring check; use equality. This can mask errors and is non-idiomatic.- if data_type in "TIMESTAMP": + if data_type == "TIMESTAMP": return "DATETIME"
🧹 Nitpick comments (8)
tox.ini (1)
115-115: Avoid global C0302 disable; prefer splitting oversized modules or file-scoped pragmas.Disabling “too-many-lines” globally can hide genuinely bloated modules. Consider splitting large files (likely src/mysql_to_sqlite3/transporter.py) or add a file-scoped disable where necessary. Also note the earlier “disable = …” under [testenv:pylint] isn’t a valid tox key and is ignored; the [pylint] section here is the effective config.
tests/unit/test_build_create_table_sql_sqlglot_identifiers.py (2)
30-33: Silence unused-argument warnings in the test helper.Tiny nit: mark unused parameters or bind to “_” to aid linters.
- def capture_execute(sql: str, *args, **kwargs): + def capture_execute(sql: str, *_, **__): executed_sql.append(sql)
95-98: Make assertion robust to whitespace formatting.The generator emits two spaces when UNIQUE is empty. Normalise whitespace in the assertion to avoid brittle tests.
- assert ( - 'CREATE INDEX IF NOT EXISTS "ta""ble_idx" ON "ta""ble" ("na""me");' in sql - or 'CREATE INDEX IF NOT EXISTS "ta""ble_idx" ON "ta""ble" ("na""me")' in sql - ) + import re + norm = re.sub(r"\s+", " ", sql) + assert 'CREATE INDEX IF NOT EXISTS "ta""ble_idx" ON "ta""ble" ("na""me")' in normsrc/mysql_to_sqlite3/transporter.py (5)
309-320: Broad exception catch; consider narrowing/logging at debug.Catching
Exceptionhides parse issues. Suggest catchingParseError/ValueErrorand, optionally, logging unexpected exceptions at DEBUG before returning None. Behaviour unchanged.- try: - tree: Expression = parse_one(cleaned, read="mysql") - return tree.sql(dialect="sqlite") - except (ParseError, ValueError, Exception): # pylint: disable=W0718 - return None + try: + tree: Expression = parse_one(cleaned, read="mysql") + return tree.sql(dialect="sqlite") + except (ParseError, ValueError): + return None + except Exception: # pragma: no cover + logging.getLogger(cls.__name__ if hasattr(cls, "__name__") else "MySQLtoSQLite").debug( + "sqlglot failed to transpile expr: %r", expr_sql + ) + return None
348-434: Typed sqlglot CAST-based type mapping looks reasonable.Covers JSON, synonyms, and TEXT/BLOB families well. Consider caching per input type to avoid repeated parsing in large schemas.
787-794: Normalise UNIQUE spacing in CREATE INDEX to avoid double spaces.Currently emits “CREATE INDEX …” when non-unique. Trim by injecting the trailing space into the
uniquetoken.- indices += """CREATE {unique} INDEX IF NOT EXISTS {name} ON {table} ({columns});""".format( - unique="UNIQUE" if index["unique"] in {1, "1"} else "", + unique_kw = "UNIQUE " if index["unique"] in {1, "1"} else "" + indices += """CREATE {unique}INDEX IF NOT EXISTS {name} ON {table} ({columns});""".format( + unique=unique_kw, name=self._quote_sqlite_identifier(unique_index_name), table=self._quote_sqlite_identifier(table_name), columns=", ".join( self._quote_sqlite_identifier(column.strip()) for column in columns.split(",") ), )
835-856: Defensive: guard NULL ON UPDATE/DELETE values and normalise case.LEFT JOIN paths can yield NULLs; emitting “ON UPDATE None” would be invalid. Default to “NO ACTION” and upper-case for consistency.
- on_update = foreign_key["on_update"] # type: ignore[index] - on_delete = foreign_key["on_delete"] # type: ignore[index] + on_update = str(foreign_key["on_update"] or "NO ACTION").upper() # type: ignore[index] + on_delete = str(foreign_key["on_delete"] or "NO ACTION").upper() # type: ignore[index]
1159-1189: Also escape backticks in data COUNT/SELECT paths.A table name with backticks breaks these queries. Reuse
_escape_mysql_backticksas in SHOW COLUMNS.- self._mysql_cur_dict.execute( + safe_table = self._escape_mysql_backticks(table_name) + self._mysql_cur_dict.execute( "SELECT COUNT(*) AS `total_records` " - f"FROM (SELECT * FROM `{table_name}` LIMIT {self._limit_rows}) AS `table`" + f"FROM (SELECT * FROM `{safe_table}` LIMIT {self._limit_rows}) AS `table`" ) @@ - self._mysql_cur_dict.execute(f"SELECT COUNT(*) AS `total_records` FROM `{table_name}`") + safe_table = self._escape_mysql_backticks(table_name) + self._mysql_cur_dict.execute(f"SELECT COUNT(*) AS `total_records` FROM `{safe_table}`") @@ - self._mysql_cur.execute( + safe_table = self._escape_mysql_backticks(table_name) + self._mysql_cur.execute( "SELECT * FROM `{table_name}` {limit}".format( - table_name=table_name, + table_name=safe_table, limit=f"LIMIT {self._limit_rows}" if self._limit_rows > 0 else "", ) )Also applies to: 1176-1184
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/mysql_to_sqlite3/transporter.py(10 hunks)tests/unit/test_build_create_table_sql_sqlglot_identifiers.py(1 hunks)tests/unit/test_collation_sqlglot_augmented.py(1 hunks)tests/unit/test_defaults_sqlglot_enhanced.py(1 hunks)tests/unit/test_types_and_defaults_extra.py(1 hunks)tests/unit/test_types_sqlglot_augmented.py(1 hunks)tox.ini(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
tests/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use pytest; keep tests under tests/ mirroring src/mysql_to_sqlite3/ structure
Files:
tests/unit/test_types_sqlglot_augmented.pytests/unit/test_defaults_sqlglot_enhanced.pytests/unit/test_types_and_defaults_extra.pytests/unit/test_build_create_table_sql_sqlglot_identifiers.pytests/unit/test_collation_sqlglot_augmented.py
tests/unit/test_*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
tests/unit/test_*.py: Add unit tests in tests/unit/ for isolated helpers; create targeted files named test_.py
Add at least one unit test covering new flag behavior/validation
Add unit tests for new type mappings in _translate_type_from_mysql_to_sqlite
Add unit tests for new default expression translations
Files:
tests/unit/test_types_sqlglot_augmented.pytests/unit/test_defaults_sqlglot_enhanced.pytests/unit/test_types_and_defaults_extra.pytests/unit/test_build_create_table_sql_sqlglot_identifiers.pytests/unit/test_collation_sqlglot_augmented.py
{src,tests}/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Adhere to lint suite: Black line length 120 and isort profile=black import ordering for all Python files
Files:
tests/unit/test_types_sqlglot_augmented.pytests/unit/test_defaults_sqlglot_enhanced.pytests/unit/test_types_and_defaults_extra.pytests/unit/test_build_create_table_sql_sqlglot_identifiers.pysrc/mysql_to_sqlite3/transporter.pytests/unit/test_collation_sqlglot_augmented.py
src/mysql_to_sqlite3/transporter.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
_
src/mysql_to_sqlite3/transporter.py: Implement and modify core transfer logic (schema introspection, batching, type/default translation, reconnection handling) in transporter.py
Convert AUTO_INCREMENT single primary keys to INTEGER PRIMARY KEY AUTOINCREMENT only when the translated type is integer; log a warning otherwise
Avoid index naming collisions: if an index name equals its table name or --prefix-indices is set, prefix with
Centralize default value translation in _translate_default_from_mysql_to_sqlite; extend this function for new MySQL constructs
Handle JSON columns: if SQLite has JSON1, map MySQL JSON to JSON; otherwise map to TEXT unless --json-as-text is set
Skip foreign key generation when a table subset restriction is applied
Use _setup_logger for logging; do not instantiate new loggers ad hoc
For large tables, prefer chunked transfer using fetchmany(self._chunk_size) and executemany inserts
On CR_SERVER_LOST during schema or data transfer, attempt a single reconnect; preserve this behavior
Disable PRAGMA foreign_keys during bulk load and re-enable in finally; ensure early returns still re-enable
Use INSERT OR IGNORE to handle potential duplicate inserts
Thread new CLI parameters through the MySQLtoSQLite constructor
Raise ValueError in constructor for invalid configuration
When --debug is not set, swallow and log errors; when --debug is set, re-raise for stack inspection
Extend _translate_type_from_mysql_to_sqlite when adding type mappings and add corresponding tests
Extend _translate_default_from_mysql_to_sqlite to support new default expressions and add tests
Centralize progress/UI changes around tqdm and honor the --quiet flagFiles:
src/mysql_to_sqlite3/transporter.pysrc/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Keep mypy passing (Python 3.9 baseline); avoid untyped dynamic attributes in source code
Files:
src/mysql_to_sqlite3/transporter.pysrc/mysql_to_sqlite3/{cli.py,transporter.py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/mysql_to_sqlite3/{cli.py,transporter.py}: Never log raw passwords
Support --skip-ssl to disable MySQL SSL; default to encrypted where possible and do not silently change defaultsFiles:
src/mysql_to_sqlite3/transporter.py🧠 Learnings (7)
📓 Common learnings
Learnt from: CR PR: techouse/mysql-to-sqlite3#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.926Z Learning: Applies to src/mysql_to_sqlite3/transporter.py : Extend _translate_type_from_mysql_to_sqlite when adding type mappings and add corresponding testsLearnt from: CR PR: techouse/mysql-to-sqlite3#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.926Z Learning: Applies to src/mysql_to_sqlite3/transporter.py : Extend _translate_default_from_mysql_to_sqlite to support new default expressions and add testsLearnt from: CR PR: techouse/mysql-to-sqlite3#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.926Z Learning: Applies to src/mysql_to_sqlite3/transporter.py : Centralize default value translation in _translate_default_from_mysql_to_sqlite; extend this function for new MySQL constructsLearnt from: CR PR: techouse/mysql-to-sqlite3#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.926Z Learning: Applies to tests/unit/test_*.py : Add unit tests for new type mappings in _translate_type_from_mysql_to_sqlite📚 Learning: 2025-10-18T21:08:55.926Z
Learnt from: CR PR: techouse/mysql-to-sqlite3#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.926Z Learning: Applies to tests/unit/test_*.py : Add unit tests for new type mappings in _translate_type_from_mysql_to_sqliteApplied to files:
tests/unit/test_types_sqlglot_augmented.pytests/unit/test_defaults_sqlglot_enhanced.pytests/unit/test_types_and_defaults_extra.pytests/unit/test_build_create_table_sql_sqlglot_identifiers.pytests/unit/test_collation_sqlglot_augmented.py📚 Learning: 2025-10-18T21:08:55.926Z
Learnt from: CR PR: techouse/mysql-to-sqlite3#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.926Z Learning: Applies to src/mysql_to_sqlite3/transporter.py : Extend _translate_type_from_mysql_to_sqlite when adding type mappings and add corresponding testsApplied to files:
tests/unit/test_types_sqlglot_augmented.pytests/unit/test_defaults_sqlglot_enhanced.pytests/unit/test_types_and_defaults_extra.pytests/unit/test_build_create_table_sql_sqlglot_identifiers.pysrc/mysql_to_sqlite3/transporter.pytests/unit/test_collation_sqlglot_augmented.py📚 Learning: 2025-10-18T21:08:55.926Z
Learnt from: CR PR: techouse/mysql-to-sqlite3#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.926Z Learning: Applies to src/mysql_to_sqlite3/transporter.py : Extend _translate_default_from_mysql_to_sqlite to support new default expressions and add testsApplied to files:
tests/unit/test_types_sqlglot_augmented.pytests/unit/test_defaults_sqlglot_enhanced.pytests/unit/test_types_and_defaults_extra.pysrc/mysql_to_sqlite3/transporter.py📚 Learning: 2025-10-18T21:08:55.926Z
Learnt from: CR PR: techouse/mysql-to-sqlite3#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.926Z Learning: Applies to src/mysql_to_sqlite3/transporter.py : Centralize default value translation in _translate_default_from_mysql_to_sqlite; extend this function for new MySQL constructsApplied to files:
tests/unit/test_defaults_sqlglot_enhanced.pytests/unit/test_types_and_defaults_extra.pysrc/mysql_to_sqlite3/transporter.py📚 Learning: 2025-10-18T21:08:55.926Z
Learnt from: CR PR: techouse/mysql-to-sqlite3#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.926Z Learning: Applies to tests/unit/test_*.py : Add unit tests for new default expression translationsApplied to files:
tests/unit/test_defaults_sqlglot_enhanced.py📚 Learning: 2025-10-18T21:08:55.925Z
Learnt from: CR PR: techouse/mysql-to-sqlite3#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.925Z Learning: Applies to src/mysql_to_sqlite3/transporter.py : Implement and modify core transfer logic (schema introspection, batching, type/default translation, reconnection handling) in transporter.pyApplied to files:
src/mysql_to_sqlite3/transporter.py🧬 Code graph analysis (6)
tests/unit/test_types_sqlglot_augmented.py (1)
src/mysql_to_sqlite3/transporter.py (2)
MySQLtoSQLite(44-1210)_translate_type_from_mysql_to_sqlite(238-307)tests/unit/test_defaults_sqlglot_enhanced.py (1)
src/mysql_to_sqlite3/transporter.py (1)
_translate_default_from_mysql_to_sqlite(436-573)tests/unit/test_types_and_defaults_extra.py (1)
src/mysql_to_sqlite3/transporter.py (1)
_translate_default_from_mysql_to_sqlite(436-573)tests/unit/test_build_create_table_sql_sqlglot_identifiers.py (2)
src/mysql_to_sqlite3/transporter.py (2)
MySQLtoSQLite(44-1210)_build_create_table_sql(650-864)tests/unit/mysql_to_sqlite3_test.py (1)
fetchall(560-561)src/mysql_to_sqlite3/transporter.py (1)
src/mysql_to_sqlite3/sqlite_utils.py (1)
CollatingSequences(51-56)tests/unit/test_collation_sqlglot_augmented.py (2)
src/mysql_to_sqlite3/sqlite_utils.py (1)
CollatingSequences(51-56)src/mysql_to_sqlite3/transporter.py (1)
_data_type_collation_sequence(576-614)🪛 Flake8 (7.3.0)
src/mysql_to_sqlite3/transporter.py
[error] 341-341: SyntaxError: f-string expression part cannot include a backslash
(E999)
🪛 Ruff (0.14.0)
tests/unit/test_build_create_table_sql_sqlglot_identifiers.py
30-30: Unused function argument:
args(ARG001)
30-30: Unused function argument:
kwargs(ARG001)
src/mysql_to_sqlite3/transporter.py
319-319: Do not catch blind exception:
Exception(BLE001)
341-341: Cannot use an escape sequence (backslash) in f-strings on Python 3.9 (syntax was added in Python 3.12)
(invalid-syntax)
341-341: Cannot use an escape sequence (backslash) in f-strings on Python 3.9 (syntax was added in Python 3.12)
(invalid-syntax)
341-341: Cannot use an escape sequence (backslash) in f-strings on Python 3.9 (syntax was added in Python 3.12)
(invalid-syntax)
363-363: Do not catch blind exception:
Exception(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Test (python3.11, mariadb:10.0, 1, true, 3.11)
- GitHub Check: Test (python3.10, mariadb:10.0, 1, true, 3.10)
- GitHub Check: Test (python3.10, mariadb:5.5, 1, true, 3.10)
- GitHub Check: Test (python3.13, mariadb:5.5, 1, true, 3.13)
- GitHub Check: Test (python3.9, mariadb:10.0, 1, true, 3.9)
- GitHub Check: Test (python3.9, mariadb:5.5, 1, true, 3.9)
- GitHub Check: Test (python3.11, mariadb:5.5, 1, true, 3.11)
- GitHub Check: Test (python3.12, mariadb:5.5, 1, true, 3.12)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (9)
tests/unit/test_collation_sqlglot_augmented.py (1)
8-17: Collation tests look solid.Good coverage for char varying/national variants, JSON exclusion, BINARY collation bypass, and numeric synonyms. LGTM.
Also applies to: 19-36, 38-51
tests/unit/test_defaults_sqlglot_enhanced.py (1)
1-60: Great coverage for default translations.The cases exercise generated defaults, boolean handling, literal normalisation, and blob literals well. LGTM.
If you want, we can add a case for parenthesised arithmetic like "(1+2*3)" to harden the path.tests/unit/test_types_and_defaults_extra.py (2)
43-49: CURRENT_TIMESTAMP(6) normalisation test is on point.Confirms sqlglot-based normalisation to SQLite token. LGTM.
50-54: Safe fallback to quoted string for unknown generated expr.Good safety check for e.g. uuid() defaults. LGTM.
Consider adding one more for an unsupported function with nested parentheses to ensure the same fallback applies.tests/unit/test_types_sqlglot_augmented.py (1)
7-72: Type translator coverage is comprehensive.Good assertions for synonyms, unsigned forms, JSON, binaries, and fallback. LGTM.
You might add a CASE for “timestamp” vs “TIMESTAMP” to ensure case-insensitive mapping remains stable.src/mysql_to_sqlite3/transporter.py (4)
272-281: Synonym mappings look good.Handling for DOUBLE PRECISION, FIXED, and NATIONAL/CHAR VARYING forms is clear and deterministic. LGTM.
Ensure tests cover “NATIONAL VARCHAR(…)” (they do).
301-307: sqlglot type fallback integration is a solid safety net.Good last-resort mapping prior to falling back to TEXT. LGTM.
579-614: Collation selection augmented via transpiler is well-scoped.Nice balance of fast-path checks and fallback mapping; avoids JSON/BLOB. LGTM.
651-658: Good: consistent quoting and safe SHOW COLUMNS lookup.Using
_quote_sqlite_identifierfor SQLite DDL and_escape_mysql_backticksfor MySQL queries is the right split. LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/mysql_to_sqlite3/transporter.py (2)
309-325: Consider narrowing exception scope or documenting rationale.The broad
Exceptioncatch at line 321 is flagged by static analysis. Whilst acceptable for a fallback transpiler, consider:
- Catching only
Exceptionsubclasses likely to occur (e.g.,AttributeError,TypeError,RuntimeError)- Or document why catching all exceptions is necessary (e.g., unknown sqlglot internal errors)
The
hasattr(cls, "__name__")check at line 322 is defensive but unnecessary—__name__always exists for classes.- except Exception: # pragma: no cover - logging.getLogger(cls.__name__ if hasattr(cls, "__name__") else "MySQLtoSQLite").debug( + except Exception: # pragma: no cover - catch-all for unknown sqlglot errors + logging.getLogger(cls.__name__).debug( "sqlglot failed to transpile expr: %r", expr_sql )
542-579: LGTM: Enhanced default translation with sqlglot integration.The integration of
_transpile_mysql_expr_to_sqliteforDEFAULT_GENERATEDdefaults improves correctness for expressions, booleans, and blob literals. The extensive conditionals appropriately handle various SQLite version requirements and literal formats.Minor duplication: the escape logic at lines 573-574 and 577-578 could be extracted to a small helper, but this is a nitpick given the clarity of the current implementation.
Based on coding guidelines
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/mysql_to_sqlite3/transporter.py(12 hunks)tests/unit/test_build_create_table_sql_sqlglot_identifiers.py(1 hunks)tests/unit/test_indices_prefix_and_uniqueness.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/unit/test_build_create_table_sql_sqlglot_identifiers.py
🧰 Additional context used
📓 Path-based instructions (6)
tests/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use pytest; keep tests under tests/ mirroring src/mysql_to_sqlite3/ structure
Files:
tests/unit/test_indices_prefix_and_uniqueness.py
tests/unit/test_*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
tests/unit/test_*.py: Add unit tests in tests/unit/ for isolated helpers; create targeted files named test_.py
Add at least one unit test covering new flag behavior/validation
Add unit tests for new type mappings in _translate_type_from_mysql_to_sqlite
Add unit tests for new default expression translations
Files:
tests/unit/test_indices_prefix_and_uniqueness.py
{src,tests}/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Adhere to lint suite: Black line length 120 and isort profile=black import ordering for all Python files
Files:
tests/unit/test_indices_prefix_and_uniqueness.pysrc/mysql_to_sqlite3/transporter.py
src/mysql_to_sqlite3/transporter.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
_
src/mysql_to_sqlite3/transporter.py: Implement and modify core transfer logic (schema introspection, batching, type/default translation, reconnection handling) in transporter.py
Convert AUTO_INCREMENT single primary keys to INTEGER PRIMARY KEY AUTOINCREMENT only when the translated type is integer; log a warning otherwise
Avoid index naming collisions: if an index name equals its table name or --prefix-indices is set, prefix with
Centralize default value translation in _translate_default_from_mysql_to_sqlite; extend this function for new MySQL constructs
Handle JSON columns: if SQLite has JSON1, map MySQL JSON to JSON; otherwise map to TEXT unless --json-as-text is set
Skip foreign key generation when a table subset restriction is applied
Use _setup_logger for logging; do not instantiate new loggers ad hoc
For large tables, prefer chunked transfer using fetchmany(self._chunk_size) and executemany inserts
On CR_SERVER_LOST during schema or data transfer, attempt a single reconnect; preserve this behavior
Disable PRAGMA foreign_keys during bulk load and re-enable in finally; ensure early returns still re-enable
Use INSERT OR IGNORE to handle potential duplicate inserts
Thread new CLI parameters through the MySQLtoSQLite constructor
Raise ValueError in constructor for invalid configuration
When --debug is not set, swallow and log errors; when --debug is set, re-raise for stack inspection
Extend _translate_type_from_mysql_to_sqlite when adding type mappings and add corresponding tests
Extend _translate_default_from_mysql_to_sqlite to support new default expressions and add tests
Centralize progress/UI changes around tqdm and honor the --quiet flagFiles:
src/mysql_to_sqlite3/transporter.pysrc/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Keep mypy passing (Python 3.9 baseline); avoid untyped dynamic attributes in source code
Files:
src/mysql_to_sqlite3/transporter.pysrc/mysql_to_sqlite3/{cli.py,transporter.py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/mysql_to_sqlite3/{cli.py,transporter.py}: Never log raw passwords
Support --skip-ssl to disable MySQL SSL; default to encrypted where possible and do not silently change defaultsFiles:
src/mysql_to_sqlite3/transporter.py🧠 Learnings (5)
📓 Common learnings
Learnt from: CR PR: techouse/mysql-to-sqlite3#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.926Z Learning: Applies to src/mysql_to_sqlite3/transporter.py : Extend _translate_type_from_mysql_to_sqlite when adding type mappings and add corresponding testsLearnt from: CR PR: techouse/mysql-to-sqlite3#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.925Z Learning: Applies to src/mysql_to_sqlite3/transporter.py : Implement and modify core transfer logic (schema introspection, batching, type/default translation, reconnection handling) in transporter.pyLearnt from: CR PR: techouse/mysql-to-sqlite3#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.926Z Learning: Applies to src/mysql_to_sqlite3/transporter.py : Extend _translate_default_from_mysql_to_sqlite to support new default expressions and add testsLearnt from: CR PR: techouse/mysql-to-sqlite3#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.926Z Learning: Applies to src/mysql_to_sqlite3/transporter.py : Centralize default value translation in _translate_default_from_mysql_to_sqlite; extend this function for new MySQL constructsLearnt from: CR PR: techouse/mysql-to-sqlite3#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.926Z Learning: Applies to tests/unit/test_*.py : Add unit tests for new type mappings in _translate_type_from_mysql_to_sqlite📚 Learning: 2025-10-18T21:08:55.926Z
Learnt from: CR PR: techouse/mysql-to-sqlite3#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.926Z Learning: Applies to src/mysql_to_sqlite3/transporter.py : Extend _translate_type_from_mysql_to_sqlite when adding type mappings and add corresponding testsApplied to files:
src/mysql_to_sqlite3/transporter.py📚 Learning: 2025-10-18T21:08:55.926Z
Learnt from: CR PR: techouse/mysql-to-sqlite3#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.926Z Learning: Applies to src/mysql_to_sqlite3/transporter.py : Extend _translate_default_from_mysql_to_sqlite to support new default expressions and add testsApplied to files:
src/mysql_to_sqlite3/transporter.py📚 Learning: 2025-10-18T21:08:55.925Z
Learnt from: CR PR: techouse/mysql-to-sqlite3#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.925Z Learning: Applies to src/mysql_to_sqlite3/transporter.py : Implement and modify core transfer logic (schema introspection, batching, type/default translation, reconnection handling) in transporter.pyApplied to files:
src/mysql_to_sqlite3/transporter.py📚 Learning: 2025-10-18T21:08:55.926Z
Learnt from: CR PR: techouse/mysql-to-sqlite3#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.926Z Learning: Applies to src/mysql_to_sqlite3/transporter.py : Centralize default value translation in _translate_default_from_mysql_to_sqlite; extend this function for new MySQL constructsApplied to files:
src/mysql_to_sqlite3/transporter.py🧬 Code graph analysis (1)
src/mysql_to_sqlite3/transporter.py (1)
src/mysql_to_sqlite3/sqlite_utils.py (1)
CollatingSequences(51-56)🪛 GitHub Actions: Test
src/mysql_to_sqlite3/transporter.py
[warning] 321-321: Pylint: W0718 - Catching too general exception (broad-exception-caught) in transporter.py:321.
🪛 Ruff (0.14.0)
src/mysql_to_sqlite3/transporter.py
321-321: Do not catch blind exception:
Exception(BLE001)
369-369: Do not catch blind exception:
Exception(BLE001)
1170-1171: Possible SQL injection vector through string-based query construction
(S608)
1176-1176: Possible SQL injection vector through string-based query construction
(S608)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (9)
src/mysql_to_sqlite3/transporter.py (8)
18-18: LGTM: Import addition supports new transpilation features.The
Expressionimport is correctly added to support the new sqlglot-based transpilation methods introduced in this PR.
272-281: LGTM: Type synonym mappings extended appropriately.The new mappings for
DOUBLE PRECISION,FIXED, and variousNATIONALcharacter types follow the established pattern and improve MySQL compatibility.Based on coding guidelines, ensure unit tests cover these new type mappings. The PR summary mentions tests, but verify coverage for:
DOUBLE PRECISION→DOUBLEFIXED→DECIMALCHARACTER VARYING/CHAR VARYING→VARCHARNATIONALvariants →NVARCHAR/NCHARBased on learnings
301-307: LGTM: Graceful fallback to sqlglot-based type transpilation.The fallback to
_transpile_mysql_type_to_sqlitewhen built-in mappings don't match improves robustness whilst preserving theTEXTdefault for unrecognised types.
327-347: LGTM: Robust identifier quoting with proper encoding handling.The implementation correctly:
- Handles
bytes,bytearray, andstrinputs- Uses sqlglot's
exp.to_identifierfor normalisation- Doubles embedded quotes per SQLite requirements
- Includes fallback for edge cases
349-352: LGTM: Simple and effective backtick escaping.This helper correctly escapes backticks for safe MySQL identifier quoting.
354-439: LGTM: Comprehensive type transpilation fallback.This well-documented fallback mapper:
- Uses sqlglot to parse uncommon MySQL type synonyms
- Respects JSON1 extension availability
- Normalises type names whilst preserving length suffixes
- Aligns with existing type mapping conventions
The broad exception handling at line 369 is acceptable for a last-resort fallback that returns
Noneon failure.
581-620: LGTM: Enhanced collation logic with sqlglot fallback.The updated method correctly:
- Fast-paths known textual types
- Avoids collations for JSON/BLOB
- Falls back to sqlglot-based type mapping for uncommon synonyms
- Maintains existing behaviour whilst improving coverage
657-870: LGTM: Consistent identifier quoting throughout DDL generation.The DDL construction properly:
- Quotes all SQLite identifiers using
_quote_sqlite_identifier- Escapes MySQL backticks using
_escape_mysql_backticksbefore interpolation- Handles bytes/bytearray column names consistently
- Quotes table, column, index, and foreign key components
Static analysis flags lines 1170, 1176, and 1189 as potential SQL injection vectors. These are false positives—the code uses
_escape_mysql_backticksbefore interpolation (lines 1168, 1175, 1187), which doubles backticks to prevent injection. The pattern is:safe_table = self._escape_mysql_backticks(table_name) self._mysql_cur_dict.execute(f"SELECT ... FROM `{safe_table}`")This is safe. However, for future maintainability, consider using parameterised queries where MySQL supports them (though table/column identifiers often require string interpolation).
tests/unit/test_indices_prefix_and_uniqueness.py (1)
52-52: LGTM: Test assertion updated to match corrected SQL formatting.The assertion now correctly expects single-space formatting in the generated
CREATE INDEXstatement, aligning with the production code changes intransporter.py.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/mysql_to_sqlite3/transporter.py (1)
354-439: Consider narrowing exception handling at line 369.The method provides a robust fallback for type transpilation. However, catching bare
Exceptionat line 369 is quite broad. While acceptable in a last-resort fallback, consider whetherParseErrorandValueErrorwould suffice, or document why broader catching is needed.- except (ParseError, ValueError, Exception): # pylint: disable=W0718 + except (ParseError, ValueError, AttributeError, TypeError): return NoneThat said, the overall logic is sound: the CAST wrapper approach, JSON1 handling, and synonym normalization are all well-designed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/mysql_to_sqlite3/transporter.py(12 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
src/mysql_to_sqlite3/transporter.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
_
src/mysql_to_sqlite3/transporter.py: Implement and modify core transfer logic (schema introspection, batching, type/default translation, reconnection handling) in transporter.py
Convert AUTO_INCREMENT single primary keys to INTEGER PRIMARY KEY AUTOINCREMENT only when the translated type is integer; log a warning otherwise
Avoid index naming collisions: if an index name equals its table name or --prefix-indices is set, prefix with
Centralize default value translation in _translate_default_from_mysql_to_sqlite; extend this function for new MySQL constructs
Handle JSON columns: if SQLite has JSON1, map MySQL JSON to JSON; otherwise map to TEXT unless --json-as-text is set
Skip foreign key generation when a table subset restriction is applied
Use _setup_logger for logging; do not instantiate new loggers ad hoc
For large tables, prefer chunked transfer using fetchmany(self._chunk_size) and executemany inserts
On CR_SERVER_LOST during schema or data transfer, attempt a single reconnect; preserve this behavior
Disable PRAGMA foreign_keys during bulk load and re-enable in finally; ensure early returns still re-enable
Use INSERT OR IGNORE to handle potential duplicate inserts
Thread new CLI parameters through the MySQLtoSQLite constructor
Raise ValueError in constructor for invalid configuration
When --debug is not set, swallow and log errors; when --debug is set, re-raise for stack inspection
Extend _translate_type_from_mysql_to_sqlite when adding type mappings and add corresponding tests
Extend _translate_default_from_mysql_to_sqlite to support new default expressions and add tests
Centralize progress/UI changes around tqdm and honor the --quiet flagFiles:
src/mysql_to_sqlite3/transporter.pysrc/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Keep mypy passing (Python 3.9 baseline); avoid untyped dynamic attributes in source code
Files:
src/mysql_to_sqlite3/transporter.pysrc/mysql_to_sqlite3/{cli.py,transporter.py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/mysql_to_sqlite3/{cli.py,transporter.py}: Never log raw passwords
Support --skip-ssl to disable MySQL SSL; default to encrypted where possible and do not silently change defaultsFiles:
src/mysql_to_sqlite3/transporter.py{src,tests}/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Adhere to lint suite: Black line length 120 and isort profile=black import ordering for all Python files
Files:
src/mysql_to_sqlite3/transporter.py🧠 Learnings (5)
📓 Common learnings
Learnt from: CR PR: techouse/mysql-to-sqlite3#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.926Z Learning: Applies to src/mysql_to_sqlite3/transporter.py : Extend _translate_type_from_mysql_to_sqlite when adding type mappings and add corresponding testsLearnt from: CR PR: techouse/mysql-to-sqlite3#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.925Z Learning: Applies to src/mysql_to_sqlite3/transporter.py : Implement and modify core transfer logic (schema introspection, batching, type/default translation, reconnection handling) in transporter.pyLearnt from: CR PR: techouse/mysql-to-sqlite3#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.926Z Learning: Applies to src/mysql_to_sqlite3/transporter.py : Extend _translate_default_from_mysql_to_sqlite to support new default expressions and add testsLearnt from: CR PR: techouse/mysql-to-sqlite3#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.926Z Learning: Applies to src/mysql_to_sqlite3/transporter.py : Centralize default value translation in _translate_default_from_mysql_to_sqlite; extend this function for new MySQL constructsLearnt from: CR PR: techouse/mysql-to-sqlite3#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.926Z Learning: Applies to tests/unit/test_*.py : Add unit tests for new type mappings in _translate_type_from_mysql_to_sqlite📚 Learning: 2025-10-18T21:08:55.926Z
Learnt from: CR PR: techouse/mysql-to-sqlite3#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.926Z Learning: Applies to src/mysql_to_sqlite3/transporter.py : Extend _translate_type_from_mysql_to_sqlite when adding type mappings and add corresponding testsApplied to files:
src/mysql_to_sqlite3/transporter.py📚 Learning: 2025-10-18T21:08:55.926Z
Learnt from: CR PR: techouse/mysql-to-sqlite3#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.926Z Learning: Applies to src/mysql_to_sqlite3/transporter.py : Extend _translate_default_from_mysql_to_sqlite to support new default expressions and add testsApplied to files:
src/mysql_to_sqlite3/transporter.py📚 Learning: 2025-10-18T21:08:55.925Z
Learnt from: CR PR: techouse/mysql-to-sqlite3#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.925Z Learning: Applies to src/mysql_to_sqlite3/transporter.py : Implement and modify core transfer logic (schema introspection, batching, type/default translation, reconnection handling) in transporter.pyApplied to files:
src/mysql_to_sqlite3/transporter.py📚 Learning: 2025-10-18T21:08:55.926Z
Learnt from: CR PR: techouse/mysql-to-sqlite3#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.926Z Learning: Applies to src/mysql_to_sqlite3/transporter.py : Centralize default value translation in _translate_default_from_mysql_to_sqlite; extend this function for new MySQL constructsApplied to files:
src/mysql_to_sqlite3/transporter.py🧬 Code graph analysis (1)
src/mysql_to_sqlite3/transporter.py (1)
src/mysql_to_sqlite3/sqlite_utils.py (1)
CollatingSequences(51-56)🪛 Ruff (0.14.0)
src/mysql_to_sqlite3/transporter.py
369-369: Do not catch blind exception:
Exception(BLE001)
1170-1171: Possible SQL injection vector through string-based query construction
(S608)
1176-1176: Possible SQL injection vector through string-based query construction
(S608)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Test (python3.11, mariadb:10.0, 1, true, 3.11)
- GitHub Check: Test (python3.9, mariadb:10.0, 1, true, 3.9)
- GitHub Check: Test (python3.10, mariadb:10.0, 1, true, 3.10)
- GitHub Check: Test (python3.12, mariadb:5.5, 1, true, 3.12)
- GitHub Check: Test (python3.13, mariadb:5.5, 1, true, 3.13)
- GitHub Check: Test (python3.9, mariadb:5.5, 1, true, 3.9)
- GitHub Check: Test (python3.10, mariadb:5.5, 1, true, 3.10)
- GitHub Check: Test (python3.11, mariadb:5.5, 1, true, 3.11)
🔇 Additional comments (8)
src/mysql_to_sqlite3/transporter.py (8)
272-307: LGTM! Type synonym expansions and sqlglot fallback improve robustness.The additions correctly handle MySQL type synonyms (DOUBLE PRECISION, FIXED, CHARACTER/NATIONAL VARYING variants) and preserve length information. The fallback to sqlglot-based transpilation for unrecognized types is a sound defensive measure.
Based on learnings
309-325: Well-structured transpilation method with appropriate error handling.The method cleanly handles MySQL-to-SQLite expression conversion using sqlglot, with proper exception handling for both expected parse failures and unexpected sqlglot issues.
327-347: Python 3.9 compatibility issue resolved.The method now correctly avoids backslashes in f-string expressions by pre-computing the replacement outside the f-string (lines 346-347), addressing the previous syntax error. The identifier quoting logic is sound and handles edge cases (bytes, special characters) appropriately.
349-352: LGTM! Correct MySQL backtick escaping.The method correctly implements MySQL's backtick escaping rule (doubling backticks within backtick-quoted identifiers).
542-579: Excellent enhancement of default value translation.The integration of sqlglot-based transpilation significantly improves default value handling, with proper fallbacks for expressions, booleans, blob literals, and arithmetic. The escaping logic (lines 573-579) is robust.
Based on learnings
585-620: Improved collation handling with sqlglot fallback.The method now correctly handles MySQL type synonyms through the transpiler fallback, while maintaining fast-path optimizations for common types. The logic is clear and well-commented.
657-871: Comprehensive identifier quoting throughout DDL construction.The updates consistently apply
_quote_sqlite_identifierand_escape_mysql_backticksacross table, column, index, and foreign key identifiers. This significantly improves robustness against special characters and edge cases. The foreign key handling (lines 842-863) correctly decodes and normalizes ON UPDATE/ON DELETE rules.
1168-1190: Correct backtick escaping for MySQL table names.The usage of
_escape_mysql_backticks(lines 1168, 1175, 1187) correctly protects against special characters in table names. The static analysis warnings about SQL injection at lines 1170-1171 and 1176 are false positives—the escaping method properly doubles backticks per MySQL's identifier quoting rules, and table names originate from trusted information_schema queries.
This pull request improves the robustness and correctness of MySQL-to-SQLite migrations by enhancing type translation, identifier quoting, and default value handling. It introduces new utility methods for quoting and escaping identifiers, uses
sqlglotto better map MySQL types and expressions to SQLite equivalents, and ensures that generated SQL is safe against problematic characters in identifiers (such as backticks and double quotes). The changes are validated with new unit tests.Type translation and expression handling improvements:
_transpile_mysql_expr_to_sqlite,_transpile_mysql_type_to_sqlite) usingsqlglotto more accurately convert MySQL types and scalar expressions to SQLite-compatible forms, improving fallback behavior for unknown or synonym types._translate_type_from_mysql_to_sqliteto handle additional MySQL type synonyms (e.g.,DOUBLE PRECISION,FIXED, variousNATIONALtypes), returning more appropriate SQLite types.Identifier quoting and escaping:
_quote_sqlite_identifierand_escape_mysql_backticksmethods to safely quote identifiers for SQLite and escape backticks in MySQL queries, ensuring correct handling of special characters and binary types. All SQL generation now uses these methods for table, column, and index names. [1] [2] [3] [4] [5] [6]Default value and collation handling:
_translate_default_from_mysql_to_sqliteto use the new expression transpiler, allowing more accurate translation of MySQL default values (including expressions, booleans, and blob literals) to SQLite syntax._data_type_collation_sequenceto use the transpiler for collation decisions, supporting more MySQL type synonyms and ensuring correct collation for textual types.Testing and validation:
tests/unit/test_build_create_table_sql_sqlglot_identifiers.pyto verify correct escaping and quoting of identifiers in generated SQL, especially for edge cases with backticks and double quotes.These changes significantly improve compatibility and safety when migrating MySQL schemas to SQLite, especially for databases with non-standard type names or identifiers containing special characters.