-
-
Notifications
You must be signed in to change notification settings - Fork 65
🦺 improve SQLite-to-MySQL type and default value translation logic #149
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
WalkthroughAdds a sqlglot-based normalization path for SQLite column types and uses it before legacy translation; extends default-value translation to optionally parse and rewrite SQLite expression defaults into MySQL with safe fallbacks; and adds extensive unit tests covering normalization, unsigned handling, expression-default behaviour, and many edge cases. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Caller
participant Transporter as SQLite3toMySQL
participant Normaliser as _normalize_sqlite_column_type
participant LegacyTranslator as legacy translation path
Caller->>Transporter: translate_type(column_type)
Transporter->>Normaliser: parse & normalise column_type
alt Normaliser returns different normalised_type
Normaliser-->>Transporter: normalised_type
Transporter->>LegacyTranslator: translate(normalised_type)
LegacyTranslator-->>Transporter: mysql_type
else Normaliser returns None or same
Transporter->>LegacyTranslator: translate(original column_type)
LegacyTranslator-->>Transporter: mysql_type
end
Transporter-->>Caller: mysql_type
sequenceDiagram
participant Caller as Caller
participant Transporter as _translate_default_for_mysql
participant Parser as sqlglot parser
participant Rewriter as SQLite→MySQL rewriter
Caller->>Transporter: translate_default(column_type, default_value)
alt _allow_expr_defaults == true
Transporter->>Parser: parse default_value as expression
alt parse succeeds
Parser-->>Transporter: parsed_expr
Transporter->>Rewriter: rewrite SQLite functions
Rewriter-->>Transporter: rewritten_expr
Transporter-->>Caller: converted MySQL default (rewritten_expr)
else parse fails
Transporter-->>Caller: original default_value (fallback)
end
else
Transporter-->>Caller: original behaviour / empty default
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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)
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 #149 +/- ##
==========================================
+ Coverage 92.86% 98.32% +5.46%
==========================================
Files 8 8
Lines 967 1016 +49
==========================================
+ Hits 898 999 +101
+ Misses 69 17 -52 ☔ 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: 0
🧹 Nitpick comments (4)
tests/unit/sqlite3_to_mysql_test.py (1)
1149-1160: Mirror production casing in the test helper to avoid surprisesUpper-case the injected MySQL type strings to match init behaviour and keep tests robust against casing differences.
Apply this diff:
@staticmethod def _type_instance( mysql_integer_type: str = "INT(11)", mysql_string_type: str = "VARCHAR(255)", mysql_text_type: str = "TEXT", ) -> SQLite3toMySQL: instance = SQLite3toMySQL.__new__(SQLite3toMySQL) - instance._mysql_integer_type = mysql_integer_type - instance._mysql_string_type = mysql_string_type - instance._mysql_text_type = mysql_text_type + instance._mysql_integer_type = mysql_integer_type.upper() + instance._mysql_string_type = mysql_string_type.upper() + instance._mysql_text_type = mysql_text_type.upper() return instancesrc/sqlite3_to_mysql/transporter.py (3)
332-340: Optional: add debug for normalisation decisions (guarded for tests)A tiny debug helps trace subtle type normalisations without impacting production logs.
Apply this diff:
normalized: t.Optional[str] = self._normalize_sqlite_column_type(column_type) if normalized and normalized.upper() != column_type.upper(): + # Debug visibility when normalisation alters the input (may be absent in test stubs) + logger = getattr(self, "_logger", None) + if logger: + logger.debug("Normalised SQLite column type %r -> %r", column_type, normalized) try: return self._translate_type_from_sqlite_to_mysql_legacy(normalized) except ValueError: pass return self._translate_type_from_sqlite_to_mysql_legacy(column_type)
341-374: Broaden normalisation: retry parse without UNSIGNED tokenIf the first parse fails, a second attempt sans UNSIGNED lets us still normalise params; UNSIGNED is appended later anyway.
Apply this diff:
normalized_for_parse: str = clean_type.upper().replace("UNSIGNED BIG INT", "BIGINT UNSIGNED") try: expression = sqlglot.parse_one(f"SELECT CAST(NULL AS {normalized_for_parse})", read="sqlite") except sqlglot_errors.ParseError: - return None + # Retry: strip UNSIGNED to aid parsing; we'll re-attach it below if present. + try: + no_unsigned = re.sub(r"\bUNSIGNED\b", "", normalized_for_parse).strip() + expression = sqlglot.parse_one(f"SELECT CAST(NULL AS {no_unsigned})", read="sqlite") + except sqlglot_errors.ParseError: + return NoneAs per coding guidelines.
580-592: Proactively suppress DATE/TIME “now” defaults when expressions aren’t supportedAvoids ER_INVALID_DEFAULT and a second CREATE pass by returning "" early for DATE/TIME on old servers.
Apply this diff:
- # DATE - if ( + # DATE + if ( base.startswith("DATE") and ( self.CURRENT_DATE.match(s) or self.CURRENT_TS.match(s) # map CURRENT_TIMESTAMP → CURRENT_DATE for DATE or (self.SQLITE_NOW_FUNC.match(s) and s.lower().startswith("date")) or self.STRFTIME_NOW.match(s) ) - and self._allow_expr_defaults ): - # Too old for expression defaults on DATE → fall back - return "CURRENT_DATE" + if not self._allow_expr_defaults: + return "" + return "CURRENT_DATE" - # TIME - if ( + # TIME + if ( base.startswith("TIME") and ( self.CURRENT_TIME.match(s) or self.CURRENT_TS.match(s) # map CURRENT_TIMESTAMP → CURRENT_TIME for TIME or (self.SQLITE_NOW_FUNC.match(s) and s.lower().startswith("time")) or self.STRFTIME_NOW.match(s) ) - and self._allow_expr_defaults ): - # Too old for expression defaults on TIME → fall back + if not self._allow_expr_defaults: + return "" len_match = self.COLUMN_LENGTH_PATTERN.search(column_type)Based on learnings.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/sqlite3_to_mysql/transporter.py(2 hunks)tests/unit/sqlite3_to_mysql_test.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
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
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
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
PR: techouse/sqlite3-to-mysql#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-23T18:16:08.668Z
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
📚 Learning: 2025-10-23T18:16:08.668Z
Learnt from: CR
PR: techouse/sqlite3-to-mysql#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-23T18:16:08.668Z
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
Applied to files:
src/sqlite3_to_mysql/transporter.pytests/unit/sqlite3_to_mysql_test.py
📚 Learning: 2025-10-23T18:16:08.668Z
Learnt from: CR
PR: techouse/sqlite3-to-mysql#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-23T18:16:08.668Z
Learning: Applies to src/sqlite3_to_mysql/transporter.py : Handle DEFAULT normalization in _translate_default_for_mysql; return empty string to suppress invalid defaults
Applied to files:
src/sqlite3_to_mysql/transporter.py
📚 Learning: 2025-10-23T18:16:08.668Z
Learnt from: CR
PR: techouse/sqlite3-to-mysql#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-23T18:16:08.668Z
Learning: Applies to src/sqlite3_to_mysql/transporter.py : Expose new capability booleans from __init__ of SQLite3toMySQL for downstream logic
Applied to files:
src/sqlite3_to_mysql/transporter.py
📚 Learning: 2025-10-23T18:16:08.668Z
Learnt from: CR
PR: techouse/sqlite3-to-mysql#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-23T18:16:08.668Z
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
Applied to files:
src/sqlite3_to_mysql/transporter.pytests/unit/sqlite3_to_mysql_test.py
📚 Learning: 2025-10-23T18:16:08.668Z
Learnt from: CR
PR: techouse/sqlite3-to-mysql#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-23T18:16:08.668Z
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
Applied to files:
tests/unit/sqlite3_to_mysql_test.py
🧬 Code graph analysis (1)
tests/unit/sqlite3_to_mysql_test.py (1)
src/sqlite3_to_mysql/transporter.py (2)
_translate_default_for_mysql(464-593)_translate_type_from_sqlite_to_mysql(332-339)
⏰ 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.12, mariadb:5.5, 1, true, 3.12)
- GitHub Check: Test (python3.11, mariadb:5.5, 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.9, mariadb:10.0, 1, true, 3.9)
- GitHub Check: Test (python3.13, mariadb:5.5, 1, true, 3.13)
- GitHub Check: Test (python3.11, mariadb:10.0, 1, true, 3.11)
- GitHub Check: Test (python3.9, mariadb:5.5, 1, true, 3.9)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (4)
tests/unit/sqlite3_to_mysql_test.py (4)
1212-1216: LGTM: strftime('…','now','utc') → DATE_FORMAT(UTC_TIMESTAMP(), …)Covers the sqlglot rewrite path for UTC modifier correctly.
1217-1221: LGTM: gated by expression‑default supportVerifies fallback to original when expressions aren’t allowed.
1222-1227: LGTM: spacing/parameter normalisationNUMERIC/CHAR/VARCHAR spacing is normalised as intended before legacy translation.
1228-1231: LGTM: preserves UNSIGNED across normalisationEnsures numeric UNSIGNED survives the normalise→legacy pipeline.
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
🧹 Nitpick comments (2)
tests/unit/sqlite3_to_mysql_test.py (2)
1210-1227: LGTM! Comprehensive spacing normalization test coverage.The test validates that sqlglot-based normalization correctly handles various spacing patterns in SQLite types (NUMERIC, VARCHAR, CHAR), ensuring consistent parameter formatting.
Optional: Address unused parameter warning.
Static analysis flags
mysql_databaseas unused. Whilst it's required for fixture setup/teardown, you could prefix it with an underscore to indicate intentional non-use:def test_translate_type_from_sqlite_to_mysql_sqlglot_normalizes_spacing( self, sqlite_database: str, - mysql_database: Engine, + _mysql_database: Engine, mysql_credentials: MySQLCredentials, ) -> None:
1229-1244: LGTM! Validates UNSIGNED modifier preservation.The test correctly verifies that the normalization path preserves the UNSIGNED modifier whilst also normalizing spacing and type names.
Optional: Address unused parameter warning.
Static analysis flags
mysql_databaseas unused. Consider prefixing with an underscore:def test_translate_type_from_sqlite_to_mysql_sqlglot_preserves_unsigned( self, sqlite_database: str, - mysql_database: Engine, + _mysql_database: Engine, mysql_credentials: MySQLCredentials, ):
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/sqlite3_to_mysql/transporter.py(4 hunks)tests/unit/sqlite3_to_mysql_test.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
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
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
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
PR: techouse/sqlite3-to-mysql#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-23T18:16:08.668Z
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
PR: techouse/sqlite3-to-mysql#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-23T18:16:08.668Z
Learning: Applies to src/sqlite3_to_mysql/transporter.py : Handle DEFAULT normalization in _translate_default_for_mysql; return empty string to suppress invalid defaults
📚 Learning: 2025-10-23T18:16:08.668Z
Learnt from: CR
PR: techouse/sqlite3-to-mysql#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-23T18:16:08.668Z
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
Applied to files:
src/sqlite3_to_mysql/transporter.pytests/unit/sqlite3_to_mysql_test.py
📚 Learning: 2025-10-23T18:16:08.668Z
Learnt from: CR
PR: techouse/sqlite3-to-mysql#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-23T18:16:08.668Z
Learning: Applies to src/sqlite3_to_mysql/transporter.py : Handle DEFAULT normalization in _translate_default_for_mysql; return empty string to suppress invalid defaults
Applied to files:
src/sqlite3_to_mysql/transporter.py
📚 Learning: 2025-10-23T18:16:08.668Z
Learnt from: CR
PR: techouse/sqlite3-to-mysql#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-23T18:16:08.668Z
Learning: Applies to src/sqlite3_to_mysql/transporter.py : Expose new capability booleans from __init__ of SQLite3toMySQL for downstream logic
Applied to files:
src/sqlite3_to_mysql/transporter.py
📚 Learning: 2025-10-23T18:16:08.668Z
Learnt from: CR
PR: techouse/sqlite3-to-mysql#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-23T18:16:08.668Z
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
Applied to files:
src/sqlite3_to_mysql/transporter.py
📚 Learning: 2025-10-23T18:16:08.668Z
Learnt from: CR
PR: techouse/sqlite3-to-mysql#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-23T18:16:08.668Z
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
Applied to files:
tests/unit/sqlite3_to_mysql_test.py
🧬 Code graph analysis (2)
src/sqlite3_to_mysql/transporter.py (1)
src/sqlite3_to_mysql/debug_info.py (1)
info(71-93)
tests/unit/sqlite3_to_mysql_test.py (1)
src/sqlite3_to_mysql/transporter.py (2)
_translate_default_for_mysql(470-601)_translate_type_from_sqlite_to_mysql(332-340)
🪛 Ruff (0.14.1)
tests/unit/sqlite3_to_mysql_test.py
1213-1213: Unused method argument: mysql_database
(ARG002)
1232-1232: Unused method argument: mysql_database
(ARG002)
⏰ 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.10, mariadb:10.0, 1, true, 3.10)
- GitHub Check: Test (python3.12, mariadb:5.5, 1, true, 3.12)
- GitHub Check: Test (python3.9, mariadb:10.0, 1, true, 3.9)
- GitHub Check: Test (python3.10, mariadb:5.5, 1, true, 3.10)
- GitHub Check: Test (python3.11, mariadb:10.0, 1, true, 3.11)
- 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.11, mariadb:5.5, 1, true, 3.11)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (4)
src/sqlite3_to_mysql/transporter.py (3)
332-340: LGTM! Normalization-first translation path is well-structured.The flow is clear: attempt sqlglot-based normalization, translate the normalized type if it differs, and fall back gracefully to the legacy path. Logging the normalization aids debugging.
342-379: LGTM! Robust normalization with comprehensive error handling.The implementation handles edge cases well: empty types, parse failures with UNSIGNED retry, parameter extraction, and UNSIGNED preservation. The defensive approach reduces fragility when encountering unexpected SQLite type syntax.
588-599: LGTM! Expression default translation with appropriate fallback.The implementation correctly leverages sqlglot and
_rewrite_sqlite_view_functionsto translate SQLite expression defaults to MySQL equivalents, with graceful fallback on parse or conversion errors.tests/unit/sqlite3_to_mysql_test.py (1)
1200-1208: LGTM! Thorough tests for expression default translation.These tests properly validate the new sqlglot-based default translation: Line 1200-1203 confirms UTC modifier handling in strftime expressions, whilst Line 1205-1208 ensures the feature flag requirement is respected.
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/sqlite3_to_mysql/transporter.py (1)
342-379: Consider adding a docstring for consistency.The normalisation logic is solid—using sqlglot to parse
CAST(NULL AS <type>)is a clever way to standardise SQLite column types. The two-pass parsing (with and without UNSIGNED) handles edge cases well, and the parameter extraction normalises spacing effectively.However, for consistency with other methods like
_translate_default_for_mysql(which has a comprehensive docstring at lines 470–477), consider adding a docstring here to document the method's purpose, approach, and return semantics (especially thatNonesignals a normalisation failure).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/sqlite3_to_mysql/transporter.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
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 (5)
📓 Common learnings
Learnt from: CR
PR: techouse/sqlite3-to-mysql#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-23T18:16:08.668Z
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
PR: techouse/sqlite3-to-mysql#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-23T18:16:08.668Z
Learning: Applies to src/sqlite3_to_mysql/transporter.py : Handle DEFAULT normalization in _translate_default_for_mysql; return empty string to suppress invalid defaults
📚 Learning: 2025-10-23T18:16:08.668Z
Learnt from: CR
PR: techouse/sqlite3-to-mysql#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-23T18:16:08.668Z
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
Applied to files:
src/sqlite3_to_mysql/transporter.py
📚 Learning: 2025-10-23T18:16:08.668Z
Learnt from: CR
PR: techouse/sqlite3-to-mysql#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-23T18:16:08.668Z
Learning: Applies to src/sqlite3_to_mysql/transporter.py : Handle DEFAULT normalization in _translate_default_for_mysql; return empty string to suppress invalid defaults
Applied to files:
src/sqlite3_to_mysql/transporter.py
📚 Learning: 2025-10-23T18:16:08.668Z
Learnt from: CR
PR: techouse/sqlite3-to-mysql#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-23T18:16:08.668Z
Learning: Applies to src/sqlite3_to_mysql/transporter.py : Expose new capability booleans from __init__ of SQLite3toMySQL for downstream logic
Applied to files:
src/sqlite3_to_mysql/transporter.py
📚 Learning: 2025-10-23T18:16:08.668Z
Learnt from: CR
PR: techouse/sqlite3-to-mysql#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-23T18:16:08.668Z
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
Applied to files:
src/sqlite3_to_mysql/transporter.py
🧬 Code graph analysis (1)
src/sqlite3_to_mysql/transporter.py (1)
src/sqlite3_to_mysql/debug_info.py (1)
info(71-93)
⏰ 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:5.5, 1, true, 3.11)
- GitHub Check: Test (python3.13, mariadb:5.5, 1, true, 3.13)
- GitHub Check: Test (python3.11, mariadb:10.0, 1, true, 3.11)
- GitHub Check: Test (python3.10, mariadb:5.5, 1, true, 3.10)
- GitHub Check: Test (python3.9, mariadb:10.0, 1, true, 3.9)
- GitHub Check: Test (python3.12, mariadb:5.5, 1, true, 3.12)
- GitHub Check: Test (python3.10, mariadb:10.0, 1, true, 3.10)
- GitHub Check: Test (python3.9, mariadb:5.5, 1, true, 3.9)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (2)
src/sqlite3_to_mysql/transporter.py (2)
332-340: Well-structured defensive normalisation flow.The approach of attempting normalisation, then trying the legacy translator with the normalised type, and finally falling back to the original type is solid and maintains backward compatibility. The logging at line 335 provides useful visibility into when normalisations occur.
586-598: Expression default fallback is well-designed.The new fallback logic for expression defaults when
_allow_expr_defaultsis true is sound. Parsing the SQLite default expression, applying view function rewrites for consistency, and converting to MySQL dialect with appropriate error handling at each stage is a robust approach. Returning the original string on parse or conversion failures preserves existing behaviour and ensures graceful degradation.Based on learnings.
This pull request introduces improvements to the SQLite-to-MySQL type and default value translation logic in
src/sqlite3_to_mysql/transporter.py. The main changes focus on normalizing and parsing SQLite column types more robustly before conversion, and on supporting expression defaults for MySQL when allowed.Type normalization and translation improvements:
_normalize_sqlite_column_typemethod to robustly parse and normalize SQLite column types, handling cases likeUNSIGNED BIG INTand extracting type parameters. This helps ensure more accurate type mapping to MySQL._translate_type_from_sqlite_to_mysqlto use the normalized type when possible, logging normalization actions and falling back gracefully if normalization or translation fails.Default value translation enhancements:
_translate_default_for_mysqlto support expression defaults if_allow_expr_defaultsis enabled. It now parses SQLite expressions, rewrites view functions, and generates MySQL-compatible SQL, with error handling for parsing failures.