🦺 enhance handling of MySQL string literals and default values #115
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
WalkthroughAdded sqlglot-based normalisation for MySQL literals and identifiers in the transporter: new classmethod for literal normalisation, default translation now attempts sqlglot normalisation before escaping, and identifier quoting uses sqlglot preprocessing. A unit test for pre-quoted string defaults was added. Changes
Sequence Diagram(s)sequenceDiagram
participant Transporter
participant Translate as _translate_default_from_mysql_to_sqlite
participant Normalize as _normalize_literal_with_sqlglot
participant SQLGlot as sqlglot
participant Fallback as Escape/Quote
participant SQLite as SQLite DEFAULT
Transporter->>Translate: provide MySQL default expression
alt default looks literal-like or parenthesised
Translate->>Normalize: pass expr_sql
Normalize->>SQLGlot: parse & attempt conversion
alt sqlglot returns literal
SQLGlot-->>Normalize: normalized literal
Normalize-->>Translate: normalized value
else sqlglot fails
SQLGlot-->>Normalize: error/None
Normalize-->>Translate: None
end
end
alt got normalized value
Translate-->>SQLite: use normalized DEFAULT
else
Translate->>Fallback: escape or quote original default
Fallback-->>SQLite: escaped DEFAULT
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
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. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #115 +/- ##
==========================================
- Coverage 99.11% 98.81% -0.31%
==========================================
Files 8 8
Lines 908 925 +17
==========================================
+ Hits 900 914 +14
- Misses 8 11 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/unit/test_types_and_defaults_extra.py (1)
88-92: LGTM! Well-targeted test for pre-quoted defaults.This test directly addresses the MariaDB pre-quoted default issue from #91, verifying that string literals with brackets, doubled quotes, and backslash-escaped quotes are correctly normalised to SQLite syntax.
Optionally, consider testing additional edge cases:
- Empty string:
"''"- Parenthesized literals:
"('value')"- Literals with special characters:
"'tab\there'"These would provide more comprehensive coverage, though the current tests adequately validate the core fix.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/mysql_to_sqlite3/transporter.py(2 hunks)tests/unit/test_types_and_defaults_extra.py(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_and_defaults_extra.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_and_defaults_extra.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_and_defaults_extra.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 (15)
📓 Common learnings
Learnt from: CR Repo: techouse/mysql-to-sqlite3 PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.973Z 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 Repo: techouse/mysql-to-sqlite3 PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.973Z 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 Repo: techouse/mysql-to-sqlite3 PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.973Z 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 Repo: techouse/mysql-to-sqlite3 PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.973Z 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 Repo: techouse/mysql-to-sqlite3 PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.973Z Learning: Applies to src/mysql_to_sqlite3/transporter.py : Handle JSON columns: if SQLite has JSON1, map MySQL JSON to JSON; otherwise map to TEXT unless --json-as-text is setLearnt from: CR Repo: techouse/mysql-to-sqlite3 PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.973Z Learning: Applies to src/mysql_to_sqlite3/{cli.py,transporter.py} : Support --skip-ssl to disable MySQL SSL; default to encrypted where possible and do not silently change defaultsLearnt from: CR Repo: techouse/mysql-to-sqlite3 PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.973Z Learning: Applies to tests/unit/test_*.py : Add unit tests for new type mappings in _translate_type_from_mysql_to_sqliteLearnt from: CR Repo: techouse/mysql-to-sqlite3 PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.973Z Learning: Applies to mysql_utils.py : Use mysql_utils.py for charset/collation sets and MySQL value adaptation; add related helpers hereLearnt from: CR Repo: techouse/mysql-to-sqlite3 PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.973Z Learning: Applies to src/mysql_to_sqlite3/types.py : Keep type hints exhaustive in types.py and update when MySQLtoSQLite constructor kwargs changeLearnt from: CR Repo: techouse/mysql-to-sqlite3 PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.973Z Learning: Applies to sqlite_utils.py : Use sqlite_utils.py for SQLite capability detection and value adaptation; add related helpers hereLearnt from: CR Repo: techouse/mysql-to-sqlite3 PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.973Z Learning: Applies to src/mysql_to_sqlite3/types.py : When constructor kwargs change, update types.py accordingly for mypy📚 Learning: 2025-10-18T21:08:55.973Z
Learnt from: CR Repo: techouse/mysql-to-sqlite3 PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.973Z 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_and_defaults_extra.pysrc/mysql_to_sqlite3/transporter.py📚 Learning: 2025-10-18T21:08:55.973Z
Learnt from: CR Repo: techouse/mysql-to-sqlite3 PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.973Z 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_and_defaults_extra.py📚 Learning: 2025-10-18T21:08:55.973Z
Learnt from: CR Repo: techouse/mysql-to-sqlite3 PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.973Z 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_types_and_defaults_extra.pysrc/mysql_to_sqlite3/transporter.py📚 Learning: 2025-10-18T21:08:55.973Z
Learnt from: CR Repo: techouse/mysql-to-sqlite3 PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.973Z 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_and_defaults_extra.pysrc/mysql_to_sqlite3/transporter.py📚 Learning: 2025-10-18T21:08:55.973Z
Learnt from: CR Repo: techouse/mysql-to-sqlite3 PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.973Z Learning: Applies to tests/unit/test_*.py : Add unit tests for new default expression translationsApplied to files:
tests/unit/test_types_and_defaults_extra.py📚 Learning: 2025-10-18T21:08:55.973Z
Learnt from: CR Repo: techouse/mysql-to-sqlite3 PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.973Z Learning: Applies to tests/** : Use pytest; keep tests under tests/ mirroring src/mysql_to_sqlite3/ structureApplied to files:
tests/unit/test_types_and_defaults_extra.py📚 Learning: 2025-10-18T21:08:55.973Z
Learnt from: CR Repo: techouse/mysql-to-sqlite3 PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.973Z Learning: Applies to src/mysql_to_sqlite3/{cli.py,transporter.py} : Support --skip-ssl to disable MySQL SSL; default to encrypted where possible and do not silently change defaultsApplied to files:
tests/unit/test_types_and_defaults_extra.pysrc/mysql_to_sqlite3/transporter.py📚 Learning: 2025-10-18T21:08:55.973Z
Learnt from: CR Repo: techouse/mysql-to-sqlite3 PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.973Z Learning: Applies to src/mysql_to_sqlite3/types.py : Keep type hints exhaustive in types.py and update when MySQLtoSQLite constructor kwargs changeApplied to files:
tests/unit/test_types_and_defaults_extra.pysrc/mysql_to_sqlite3/transporter.py📚 Learning: 2025-10-18T21:08:55.973Z
Learnt from: CR Repo: techouse/mysql-to-sqlite3 PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.973Z Learning: Applies to tests/unit/test_*.py : Add at least one unit test covering new flag behavior/validationApplied to files:
tests/unit/test_types_and_defaults_extra.py📚 Learning: 2025-10-18T21:08:55.973Z
Learnt from: CR Repo: techouse/mysql-to-sqlite3 PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.973Z Learning: Applies to src/mysql_to_sqlite3/transporter.py : Handle JSON columns: if SQLite has JSON1, map MySQL JSON to JSON; otherwise map to TEXT unless --json-as-text is setApplied to files:
tests/unit/test_types_and_defaults_extra.pysrc/mysql_to_sqlite3/transporter.py📚 Learning: 2025-10-18T21:08:55.973Z
Learnt from: CR Repo: techouse/mysql-to-sqlite3 PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.973Z 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.973Z
Learnt from: CR Repo: techouse/mysql-to-sqlite3 PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.973Z Learning: Applies to src/mysql_to_sqlite3/transporter.py : Thread new CLI parameters through the MySQLtoSQLite constructorApplied to files:
src/mysql_to_sqlite3/transporter.py📚 Learning: 2025-10-18T21:08:55.973Z
Learnt from: CR Repo: techouse/mysql-to-sqlite3 PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.973Z Learning: Applies to src/mysql_to_sqlite3/transporter.py : Avoid index naming collisions: if an index name equals its table name or --prefix-indices is set, prefix with <table>_Applied to files:
src/mysql_to_sqlite3/transporter.py📚 Learning: 2025-10-18T21:08:55.973Z
Learnt from: CR Repo: techouse/mysql-to-sqlite3 PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-18T21:08:55.973Z Learning: Applies to src/mysql_to_sqlite3/transporter.py : Use _setup_logger for logging; do not instantiate new loggers ad hocApplied to files:
src/mysql_to_sqlite3/transporter.py🧬 Code graph analysis (2)
tests/unit/test_types_and_defaults_extra.py (1)
src/mysql_to_sqlite3/transporter.py (2)
MySQLtoSQLite(44-1241)_translate_default_from_mysql_to_sqlite(456-601)src/mysql_to_sqlite3/transporter.py (1)
tests/unit/test_transporter.py (1)
sql(175-176)⏰ 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.13, mariadb:5.5, 1, true, 3.13)
- 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.11, mariadb:5.5, 1, true, 3.11)
- GitHub Check: Test (python3.14, mariadb:5.5, 1, true, 3.14)
- 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.10, mariadb:5.5, 1, true, 3.10)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (2)
src/mysql_to_sqlite3/transporter.py (2)
327-339: LGTM! Clean sqlglot-based literal normalization.The method correctly:
- Parses MySQL expressions with sqlglot
- Identifies literal nodes and returns SQLite-compatible SQL
- Unwraps parenthesized literals (e.g.,
('value')→'value')- Returns None for non-literal expressions, enabling fallback logic
586-592: LGTM! Robust pre-quoted default handling with fallback.This addition correctly targets non-
DEFAULT_GENERATEDdefaults that are pre-quoted or parenthesized (the MariaDB edge case from issue #91). The logic:
- Attempts sqlglot normalization for literal-like defaults
- Falls back to manual escaping if normalization fails
This ensures
'[]'is properly handled asDEFAULT '[]'in SQLite, fixing the reported syntax error whilst maintaining backward compatibility.
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.
This pull request enhances the handling of MySQL string literals and default values during migration to SQLite, improving both correctness and robustness. The main focus is on normalizing pre-quoted string literals using
sqlglotand ensuring consistent escaping for SQLite compatibility.Literal normalization and default value translation:
_normalize_literal_with_sqlglotintransporter.pyto robustly parse and normalize MySQL literals to SQLite SQL usingsqlglot. This method handles both direct literals and parenthesized literals._translate_default_from_mysql_to_sqliteto use the new normalization method for pre-quoted or parenthesized string defaults, falling back to manual escaping only if normalization fails. This improves handling of edge cases like MariaDB's pre-quoted defaults.Testing improvements:
test_types_and_defaults_extra.py, verifying correct normalization and escaping for various string formats.Fixes #91