🐛 transfer `TEXT` default #152

techouse · 2025-11-01T09:38:00Z

This pull request enhances the handling of default values for text and JSON columns when migrating from SQLite3 to MySQL/MariaDB, ensuring compatibility with both database systems. It introduces new logic to detect MariaDB, refactors how column types and defaults are processed, and adds comprehensive tests for these behaviors.

Default value handling improvements:

Added logic to detect MariaDB by inspecting the MySQL version string and storing the result in the _is_mariadb attribute. This allows for conditional formatting of default values depending on the target database. [1] [2] [3]
Refactored the process for determining whether a column type supports default values, replacing the static exclusion list with the new _column_type_supports_default method for improved flexibility and correctness. [1] [2]
Added _base_mysql_column_type, _parse_sql_expression, and _format_textual_default helper methods to normalize column types and safely format default expressions, especially for text and JSON columns. These methods handle differences between MySQL and MariaDB regarding expression defaults. [1] [2]

Testing and validation:

Added extensive unit tests in sqlite3_to_mysql_test.py to verify the new default value handling logic, including MariaDB detection, expression parsing, and default formatting for various scenarios.

Code cleanup:

Removed the unused MYSQL_COLUMN_TYPES_WITHOUT_DEFAULT import, as its logic is now handled by the new helper methods.

coderabbitai · 2025-11-01T09:38:11Z

Walkthrough

The changes introduce MariaDB detection and enhanced DEFAULT value handling for SQLite-to-MySQL schema translation. New helper methods parse and format SQL DEFAULT expressions using sqlglot, with dialect-aware logic distinguishing MariaDB from MySQL behaviour. A MariaDB flag is detected from version strings and propagated through table creation workflows.

Changes

Cohort / File(s)	Change Summary
Core transporter enhancements `src/sqlite3_to_mysql/transporter.py`	Added MariaDB detection flag (`_is_mariadb`), base type extraction method (`_base_mysql_column_type`), default support checker (`_column_type_supports_default`), SQL expression parser (`_parse_sql_expression`), and textual default formatter (`_format_textual_default`). Integrated new logic into `_create_table` to apply dialect-aware DEFAULT formatting.
Type definition expansion `src/sqlite3_to_mysql/types.py`	Added `_is_mariadb: bool` attribute to `SQLite3toMySQLAttributes` class to track MariaDB variant detection.
Unit test coverage `tests/unit/sqlite3_to_mysql_test.py`	Added comprehensive test suite covering DEFAULT handling for TEXT columns across MariaDB and MySQL variants, SQL expression parsing fallback behaviour, textual default formatting under various dialect and expression flags, column type normalisation, default support checking, and edge cases (unparseable expressions, blank/null values, parenthesis handling).

Sequence Diagram

sequenceDiagram
    participant Transporter as SQLite3toMySQL
    participant Detector as MariaDB Detector
    participant TypeCheck as Type Checker
    participant Parser as Expression Parser
    participant Formatter as Default Formatter
    
    Transporter->>Detector: Detect MariaDB from version
    Detector-->>Transporter: _is_mariadb flag
    
    loop For each column with DEFAULT
        Transporter->>TypeCheck: Check if type supports default
        alt Type supports DEFAULT
            TypeCheck-->>Transporter: true
            Transporter->>Parser: Parse default expression
            Parser-->>Transporter: Expression or None
            alt Expression parsed successfully
                Transporter->>Formatter: Format with dialect awareness
                note over Formatter: MariaDB: '...' literal<br/>MySQL: (EXPR) function style
                Formatter-->>Transporter: Formatted DEFAULT
            else Expression unparseable
                Transporter->>Formatter: Wrap in parentheses
                Formatter-->>Transporter: Formatted DEFAULT
            end
        else Type unsupported
            TypeCheck-->>Transporter: false
            note over Transporter: Omit DEFAULT clause
        end
    end

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Areas requiring careful attention:
- SQL expression parsing fallback logic (MySQL dialect first, then SQLite) and edge cases (unparseable input, whitespace-only values)
- MariaDB vs MySQL DEFAULT formatting divergence, especially for TEXT columns with JSON support
- Type classification logic in _column_type_supports_default covering GEOMETRY, BLOB, and JSON-TEXT variants
- Integration points in _create_table where new helpers are invoked with correct context variables

Possibly Related PRs

🦺 improve SQLite-to-MySQL type and default value translation logic #149: Adds sqlglot-based normalisation and expression-DEFAULT translation for SQLite→MySQL conversion; overlaps with this PR's dialect-aware DEFAULT handling approach.
✨ translate SQLite defaults to MySQL #142: Modifies DEFAULT handling in transporter.py with version/MariaDB-aware flags; directly related to MariaDB detection and conditional DEFAULT logic introduced here.

Poem

🐰 MariaDB hops through the schema with glee,
Parsing defaults with sqlglot's finesse, you'll see!
TEXT columns dance in parentheses bright,
Formatting rules set the conversion right,
One transporter to bind them—MySQL and Maria in flight! ✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description Check	⚠️ Warning	The pull request description provides substantive content covering the changes made, the motivation, and the scope of work, including detailed bullet points about default value handling improvements, testing, and code cleanup. However, the description does not follow the required template structure specified in the repository. It is missing several critical sections: no "Type of change" section with checkbox options, no "How Has This Been Tested?" section with test descriptions and reproduction instructions, and no "Checklist" section to verify code standards, self-review, documentation, tests, and other requirements. The description presents the information in a narrative format rather than the structured template format that is expected for this repository.	The pull request description should be updated to follow the repository's template structure. Please add the "Type of change" section and mark the appropriate checkbox (this appears to be a bug fix based on the PR label and content). Add a "How Has This Been Tested?" section describing the test execution and configuration. Include the "Checklist" section and verify all items accordingly, particularly confirming that unit tests pass with the changes and that the code follows project guidelines.
Docstring Coverage	⚠️ Warning	Docstring coverage is 8.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The pull request title ":bug: transfer `TEXT` default" is directly related to the main changes in the changeset. The PR's core objective is to enhance the handling of default values for TEXT and JSON columns when migrating from SQLite3 to MySQL/MariaDB, and the title accurately captures this focus on TEXT column defaults. While the title doesn't encompass all aspects of the PR (such as MariaDB detection and broader default handling refactoring), it is specific and clear enough that a teammate scanning the history would understand this is about default value handling for TEXT columns. Titles are not expected to cover every detail of the changeset, and this one appropriately highlights the primary concern.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch fix/text-default

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

❤️ Share

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

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

src/sqlite3_to_mysql/transporter.py (1)

356-395: expr.copy() usage is correct and tested; add inline comments explaining the MySQL 8.0.13+ DEFAULT wrapping logic.

The script results confirm that expr.copy() is used safely at line 394 and is covered by unit tests (specifically test_format_textual_default_preserves_existing_parens). The pattern correctly wraps copies of expressions without mutating the originals, which is standard for sqlglot AST manipulation.

The complex conditional logic—particularly the MariaDB vs MySQL parentheses wrapping at lines 377–395—handles MySQL 8.0.13+ expression DEFAULT requirements correctly. However, the logic would benefit from an inline comment clarifying why MySQL requires parentheses wrapping whilst MariaDB does not, to aid future maintainers.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 53dfc1f and 045a1e3.

📒 Files selected for processing (3)

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

🧰 Additional context used

📓 Path-based instructions (8)

src/sqlite3_to_mysql/types.py

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

Provide typed parameter/attribute structures in types.py for consumption via typing.Unpack in SQLite3toMySQL.init

Files:

src/sqlite3_to_mysql/types.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/types.py
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/types.py
src/sqlite3_to_mysql/transporter.py

src/sqlite3_to_mysql/transporter.py

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

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

Files:

src/sqlite3_to_mysql/transporter.py

src/sqlite3_to_mysql/{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 (13)

📓 Common learnings

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

Learnt from: CR
Repo: techouse/sqlite3-to-mysql PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-23T18:16:08.709Z
Learning: Applies to src/sqlite3_to_mysql/transporter.py : Handle DEFAULT normalization in _translate_default_for_mysql; return empty string to suppress invalid defaults

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

Learnt from: CR
Repo: techouse/sqlite3-to-mysql PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-23T18:16:08.709Z
Learning: Applies to src/sqlite3_to_mysql/transporter.py : On table creation failures due to expression DEFAULTs, retry without DEFAULTs (two-pass _create_table with skip_default=True)

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

Learnt from: CR
Repo: techouse/sqlite3-to-mysql PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-23T18:16:08.709Z
Learning: Applies to src/sqlite3_to_mysql/transporter.py : Initialize MySQL capability booleans (e.g., _mysql_fulltext_support, _allow_expr_defaults) early in __init__, and gate conditional behavior on them

Learnt from: CR
Repo: techouse/sqlite3-to-mysql PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-23T18:16:08.709Z
Learning: Applies to src/sqlite3_to_mysql/cli.py : Expose insert method choices IGNORE (default), UPDATE, and DEFAULT via CLI with clear help text

Learnt from: CR
Repo: techouse/sqlite3-to-mysql PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-23T18:16:08.709Z
Learning: Applies to src/sqlite3_to_mysql/cli.py : Define new CLI flags above cli() with consistent help text and error messages; maintain mutual exclusion patterns

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

Learnt from: CR
Repo: techouse/sqlite3-to-mysql PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-23T18:16:08.709Z
Learning: Applies to src/sqlite3_to_mysql/transporter.py : Implement insert methods: IGNORE (default), UPDATE using ON DUPLICATE KEY UPDATE (with optional VALUES alias), and DEFAULT (no modifiers)

Learnt from: CR
Repo: techouse/sqlite3-to-mysql PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-23T18:16:08.709Z
Learning: Applies to src/sqlite3_to_mysql/transporter.py : Expose new capability booleans from __init__ of SQLite3toMySQL for downstream logic