Fix TEXT column variants not round-tripping correctly by dereuromark · Pull Request #1032 · cakephp/migrations

dereuromark · 2026-03-01T13:07:57Z

Summary

Adds rawType-based mapping for TEXT columns in mapColumnType() to properly distinguish TINYTEXT, MEDIUMTEXT, and LONGTEXT variants
Mirrors the existing BLOB handling pattern that was already working correctly
Adds round-trip tests for TEXT column variants

Problem

When using migration_diff, TEXT column variants (TINYTEXT, MEDIUMTEXT, LONGTEXT) were not being properly mapped back from the database. The code already handled BLOB variants correctly by checking the raw MySQL type string, but TEXT variants were missing equivalent handling.

For example, a LONGTEXT column would:

Be read from the database with rawType='longtext' and length=4294967295
Not be recognized as LONGTEXT (no rawType check for TEXT)
Fall through to mapColumnData() where 4294967295 didn't match any case
Result in a plain TEXT column being created instead of LONGTEXT

Solution

Added equivalent rawType-based handling for TEXT columns in mapColumnType():

tinytext → TEXT_TINY (255)
mediumtext → TEXT_MEDIUM (16777215)
longtext → TEXT_LONG (2147483647)
Regular text → null (default TEXT)

When using migration_diff, TEXT column variants (TINYTEXT, MEDIUMTEXT, LONGTEXT) were not being properly mapped back from the database. The BLOB type handling already used rawType to distinguish BLOB variants, but TEXT variants were missing equivalent handling. This adds similar rawType-based mapping for TEXT columns in mapColumnType() and includes round-trip tests. Fixes #1029

dereuromark · 2026-03-01T13:13:34Z

CI Failures Note

The CI failures on MySQL/MariaDB are unrelated to this PR. They are pre-existing issues:

MariaDB testMigrateAndRollback failure - Test expects CURRENT_TIMESTAMP but MariaDB returns current_timestamp() (case/format difference)
PHP 8.5 BakeMigrationDiffCommandTest errors - Cake\Database\Schema\Column::$fixed property uninitialized access (CakePHP core issue)

The 5.x branch CI passed 4 days ago with the same test matrix. These failures appear to be caused by recent dependency updates or test flakiness.

My changes only modify mapColumnType() in MysqlAdapter.php to handle TEXT column variants, and the new testTextRoundTrip test is not mentioned in any failures.

dereuromark · 2026-03-01T16:53:47Z

CI failures are likely not relevant, see #1034

dereuromark · 2026-03-01T16:53:51Z

CI failures are likely not relevant, see #1034

markstory · 2026-03-03T04:03:22Z

tests/TestCase/Db/Adapter/MysqlAdapterTest.php

+            ['text', null, 'text', null],
+            ['text', MysqlAdapter::TEXT_TINY, 'text', MysqlAdapter::TEXT_TINY],
+            ['text', MysqlAdapter::TEXT_MEDIUM, 'text', MysqlAdapter::TEXT_MEDIUM],
+            ['text', MysqlAdapter::TEXT_LONG, 'text', MysqlAdapter::TEXT_LONG],


Do you need all four? Wouldn't one pair suffice?

Can it be reduced? Yes, technically. The code paths are symmetric.

Downsides of reducing:

Each is a genuinely different MySQL column type with different storage characteristics

If someone modifies the constants or the mapping, a missing test case could let a regression slip through

It's testing 4 branches in that match statement - reducing means less branch coverage

I was thinking of the duplication in the parameter lists. The number of scenarios seems required. In any case it doesn't need to be a blocker.

When using migration_diff, TEXT column variants (TINYTEXT, MEDIUMTEXT, LONGTEXT) were not being properly mapped back from the database. The BLOB type handling already used rawType to distinguish BLOB variants, but TEXT variants were missing equivalent handling. This adds similar rawType-based mapping for TEXT columns in mapColumnType() and includes round-trip tests. Fixes #1029

* add using when changing column type to json (#1031) * Fix CI failures on MySQL/MariaDB (#1034) 1. Handle uninitialized Column::$fixed property in BakeMigrationDiffCommand When TableSchema::getColumn() is called on cached/serialized schema objects, the Column::$fixed property may not be initialized, causing an Error. Added safeGetColumn() helper that catches this error and uses reflection to initialize uninitialized properties before retry. 2. Fix CURRENT_TIMESTAMP test assertion for MySQL/MariaDB Different versions of MySQL and MariaDB return CURRENT_TIMESTAMP in different formats (CURRENT_TIMESTAMP, current_timestamp(), CURRENT_TIMESTAMP()). Changed the test to use a regex that accepts all valid formats case-insensitively. Fixes #1033 * Fix TEXT column variants not round-tripping correctly (#1032) When using migration_diff, TEXT column variants (TINYTEXT, MEDIUMTEXT, LONGTEXT) were not being properly mapped back from the database. The BLOB type handling already used rawType to distinguish BLOB variants, but TEXT variants were missing equivalent handling. This adds similar rawType-based mapping for TEXT columns in mapColumnType() and includes round-trip tests. Fixes #1029 * Add fixed option for binary column type (#1014) Add support for the `fixed` attribute on binary columns to distinguish between fixed-length BINARY and variable-length VARBINARY types. This mirrors cakephp/cakephp#19207. * Add test cases for fixed option on binary columns Tests cover: - Column class getter/setter for fixed option - Column::toArray() including fixed in output - Column::setOptions() accepting fixed option - MigrationHelper::getColumnOption() including/excluding fixed - MysqlAdapter creating BINARY vs VARBINARY based on fixed option * Add column type assertions to binary fixed test * Fix misleading next steps message in upgrade command (#1037) Only show 'Set legacyTables => false' step after tables have been dropped, as the upgrade command becomes unavailable once this config is set. Fixes #1036 * Fix upgrade command not matching plugins with slashes (#1039) * Fix upgrade command not matching plugins with slashes When upgrading from legacy phinxlog tables, the plugin name was being derived by camelizing the table prefix (e.g. cake_d_c_users -> CakeDCUsers). This didn't work for plugins with slashes in their names like CakeDC/Users since the slash was replaced with underscore during table name generation. This fix builds a map of loaded plugin names to their expected table prefixes, allowing proper matching of plugins like CakeDC/Users. Fixes #1038 * Fix test for SQL Server compatibility * Add TYPE_BIT constant to AdapterInterface (#1013) * Add TYPE_BIT constant to AdapterInterface Adds the TYPE_BIT constant mapping to TableSchemaInterface::TYPE_BIT for consistency with the core BIT type support added in cakephp/cakephp#19223. This allows migration users to reference AdapterInterface::TYPE_BIT when defining BIT columns in MySQL migrations. * Bump cakephp/database constraint to ^5.3.2 for TYPE_BIT support * Bump PHPStan level +1 * Make props non-nullable * Add null guards for getConstraint() in BakeMigrationDiffCommand * Fix nullable return types in Column and ForeignKey Column::getNull() now coalesces the nullable bool property to false. ForeignKey::getOnDelete()/getOnUpdate() guard against null from getDelete()/getUpdate() before calling mapAction(). * Fix null safety in Db adapters and domain classes Cast preg_replace results to string, coalesce nullable getColumns() returns, cast nullable getReferencedTable()/getName() at call sites, and use null-safe operator for optional ConsoleIo. * Fix remaining PHPStan level 8 null safety issues Add null guards for constraint diff, fail-fast for null column type, fix import order, and remaining null safety in BaseSeed, ColumnParser, TableFinder, and MigrationHelper. * Replace assert with RuntimeException in AbstractAdapter::getConnection() Assertions can be disabled in production, which would allow a null connection to slip through silently. Throw a RuntimeException instead to fail fast in all environments. * Use truthiness checks for optional nullable getters Replace (string) casts with assign-then-check pattern for optional values that can legitimately be null: Index::getWhere(), ForeignKey::getName(), Index::getName(), Column::getGenerated(). This avoids silently converting null to empty string. * Throw on null for required Column::getName() and FK::getReferencedTable() Replace (string) casts with explicit null checks and InvalidArgumentException throws for values that must be present: Column::getName() in SQL generation and ForeignKey::getReferencedTable() in FK definition builders. A column without a name or a foreign key without a referenced table are always programming errors that should fail fast rather than silently produce broken SQL. * Narrow ForeignKey::getColumns() return type and remove null guards Override getColumns() in Migrations ForeignKey to return array instead of the parent's ?array. The $columns property is always initialized as [] in the constructor, making null unreachable. This is a covariant return type narrowing, same pattern used for Column::getNull(). Removes the now-unnecessary ?? [] fallbacks from all 6 call sites across the adapter and plan classes. * Narrow Column::getName() return type and remove dead null checks Override getName() in Migrations Column to return string instead of the parent's ?string. The $name property is typed as string (not ?string) in the constructor, making null unreachable. This is a covariant return type narrowing, same pattern used for ForeignKey::getColumns(). Removes now-unnecessary null checks across adapter/action files and the dead null guard in Table::setPrimaryKey(). * Add null guards for column diff loop in BakeMigrationDiffCommand Use early-continue pattern to narrow nullable safeGetColumn() return types before array operations, fixing PHPStan level 8 errors. * Cast nullable getName() to string for drop FK/index instructions * Fix phpcs coding standard violations Remove unused InvalidArgumentException import from RecordingAdapter, extract assignment out of if condition in PostgresAdapter, and remove blank line before closing brace in ForeignKeyTest. * Consolidate null guard conditions in BakeMigrationDiffCommand * Fix inverted condition in ChangeColumn name fallback * Accept null in Column::setFixed() for snapshot compatibility Generated migration snapshots can emit 'fixed' => null for binary columns, which causes a TypeError since setFixed() only accepted bool. Widen the parameter to ?bool to match the property and getter types. Fixes #1046. --------- Co-authored-by: Matthias Wirtz <matthias.wirtz@hotmail.de> Co-authored-by: Mark Scherer <dereuromark@users.noreply.github.com> Co-authored-by: Jamison Bryant <jbryant@ticketsauce.com>

dereuromark added this to the 5.x (CakePHP 5.x, builtin-backend) milestone Mar 1, 2026

dereuromark requested a review from markstory March 1, 2026 13:09

markstory approved these changes Mar 3, 2026

View reviewed changes

markstory approved these changes Mar 5, 2026

View reviewed changes

dereuromark marked this pull request as ready for review March 5, 2026 03:23

Merge branch '5.x' into fix-longtext-roundtrip

e12d24e

dereuromark added the bug label Mar 5, 2026

dereuromark merged commit d6720e1 into 5.x Mar 5, 2026
14 checks passed

dereuromark deleted the fix-longtext-roundtrip branch March 5, 2026 06:13

dereuromark mentioned this pull request Mar 11, 2026

migration_diff for LONGTEXT create TEXT since v5.x #1029

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix TEXT column variants not round-tripping correctly#1032

Fix TEXT column variants not round-tripping correctly#1032
dereuromark merged 2 commits into5.xfrom
fix-longtext-roundtrip

dereuromark commented Mar 1, 2026

Uh oh!

dereuromark commented Mar 1, 2026

Uh oh!

dereuromark commented Mar 1, 2026

Uh oh!

dereuromark commented Mar 1, 2026

Uh oh!

markstory Mar 3, 2026

Uh oh!

dereuromark Mar 3, 2026

Uh oh!

markstory Mar 5, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

dereuromark commented Mar 1, 2026

Summary

Problem

Solution

Uh oh!

dereuromark commented Mar 1, 2026

CI Failures Note

Uh oh!

dereuromark commented Mar 1, 2026

Uh oh!

dereuromark commented Mar 1, 2026

Uh oh!

markstory Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

dereuromark Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

markstory Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants