Skip to content

Fix up blob columns. #913

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.

Sign up for GitHub

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

Merged
markstory merged 6 commits into from
Oct 10, 2025
+93 −22
Merged

Fix up blob columns. #913

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
50dae9b
Fix up blob columns.
dereuromark Oct 5, 2025
95de838
Fix up blob columns.
dereuromark Oct 5, 2025
37e533c
Fix up blob columns.
dereuromark Oct 6, 2025
6184f87
Update test expectations for BLOB column fix
dereuromark Oct 6, 2025
cbc1836
Fix up blob columns.
dereuromark Oct 6, 2025
0782a0f
Slim down test and update docstring
markstory Oct 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions src/Db/Adapter/MysqlAdapter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,7 @@ public function truncateTable(string $tableName): void
* Convert from cakephp/database conventions to migrations\column
*
* - converts datetimefractional -> datetime + length
* - converts binary types to mysql blob type constants.
*
* @param array $columnData The cakephp/database column data to transform
* @return array The extracted/converted type and length.
Expand All @@ -469,6 +470,28 @@ protected function mapColumnType(array $columnData): array
} elseif ($type === TableSchema::TYPE_TIMESTAMP_FRACTIONAL) {
$type = 'timestamp';
$length = $columnData['precision'] ?? $length;
} elseif ($type === TableSchema::TYPE_BINARY) {
// CakePHP returns BLOB columns as 'binary' with specific lengths
// Check the raw MySQL type to distinguish BLOB from BINARY columns
$rawType = $columnData['rawType'] ?? '';
if (str_contains($rawType, 'blob')) {
// Map BLOB columns back to the appropriate BLOB types
if (str_contains($rawType, 'tinyblob')) {
$type = static::PHINX_TYPE_TINYBLOB;
$length = static::BLOB_TINY;
} elseif (str_contains($rawType, 'mediumblob')) {
$type = static::PHINX_TYPE_MEDIUMBLOB;
$length = static::BLOB_MEDIUM;
} elseif (str_contains($rawType, 'longblob')) {
$type = static::PHINX_TYPE_LONGBLOB;
$length = static::BLOB_LONG;
} else {
// Regular BLOB
$type = static::PHINX_TYPE_BLOB;
$length = static::BLOB_REGULAR;
}
}
// else: keep as binary or varbinary (actual BINARY/VARBINARY column)
}

return [$type, $length];
Expand All @@ -481,8 +504,17 @@ public function getColumns(string $tableName): array
{
$dialect = $this->getSchemaDialect();
$columnRecords = $dialect->describeColumns($tableName);

// Fetch raw column types to distinguish BLOB from BINARY columns
$rawTypes = [];
$rows = $this->fetchAll(sprintf('SHOW COLUMNS FROM %s', $this->quoteTableName($tableName)));
foreach ($rows as $row) {
$rawTypes[$row['Field']] = strtolower($row['Type']);
}

$columns = [];
foreach ($columnRecords as $record) {
$record['rawType'] = $rawTypes[$record['name']] ?? null;
[$type, $length] = $this->mapColumnType($record);

$column = (new Column())
Expand Down
83 changes: 61 additions & 22 deletions tests/TestCase/Db/Adapter/MysqlAdapterTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1069,11 +1069,12 @@ public function testTinyTextColumn()
public static function binaryToBlobAutomaticConversionData()
{
return [
// limit, expected type, expected limit
[null, 'binary', null],
[64, 'binary', 255],
// When creating binary with limit > 255, MySQL auto-converts to BLOB
// input limit, expected SQL type name, expected column limit after round-trip
[null, 'blob', MysqlAdapter::BLOB_REGULAR], // binary(null) becomes BLOB
[64, 'tinyblob', MysqlAdapter::BLOB_TINY], // binary(64) becomes TINYBLOB
[MysqlAdapter::BLOB_REGULAR - 20, 'mediumblob', MysqlAdapter::BLOB_MEDIUM],
[MysqlAdapter::BLOB_REGULAR, 'binary', null],
[MysqlAdapter::BLOB_REGULAR, 'blob', MysqlAdapter::BLOB_REGULAR],
[MysqlAdapter::BLOB_REGULAR + 20, 'mediumblob', MysqlAdapter::BLOB_MEDIUM],
[MysqlAdapter::BLOB_MEDIUM, 'mediumblob', MysqlAdapter::BLOB_MEDIUM],
[MysqlAdapter::BLOB_MEDIUM + 20, 'longblob', MysqlAdapter::BLOB_LONG],
Expand All @@ -1096,11 +1097,12 @@ public function testBinaryToBlobAutomaticConversion(?int $limit, string $expecte
public static function varbinaryToBlobAutomaticConversionData()
{
return [
// limit, expected type, expected limit
[null, 'binary', null],
[64, 'binary', 255],
// When creating varbinary with limit > 255, MySQL auto-converts to BLOB
// input limit, expected SQL type name, expected column limit after round-trip
[null, 'blob', MysqlAdapter::BLOB_REGULAR], // varbinary(null) becomes BLOB
[64, 'tinyblob', MysqlAdapter::BLOB_TINY], // varbinary(64) becomes TINYBLOB
[MysqlAdapter::BLOB_REGULAR - 20, 'mediumblob', MysqlAdapter::BLOB_MEDIUM],
[MysqlAdapter::BLOB_REGULAR, 'binary', null],
[MysqlAdapter::BLOB_REGULAR, 'blob', MysqlAdapter::BLOB_REGULAR],
[MysqlAdapter::BLOB_REGULAR + 20, 'mediumblob', MysqlAdapter::BLOB_MEDIUM],
[MysqlAdapter::BLOB_MEDIUM, 'mediumblob', MysqlAdapter::BLOB_MEDIUM],
[MysqlAdapter::BLOB_MEDIUM + 20, 'longblob', MysqlAdapter::BLOB_LONG],
Expand All @@ -1123,28 +1125,29 @@ public function testVarbinaryToBlobAutomaticConversion(?int $limit, string $expe
public static function blobColumnsData()
{
return [
// type, expected type, limit, expected limit
// BLOB columns with various limits - MySQL auto-selects appropriate BLOB subtype
// input type, expected SQL type, input limit, expected column limit after round-trip
// Tiny blobs
['tinyblob', 'binary', null, MysqlAdapter::BLOB_TINY],
['tinyblob', 'binary', MysqlAdapter::BLOB_TINY, MysqlAdapter::BLOB_TINY],
['tinyblob', 'tinyblob', null, MysqlAdapter::BLOB_TINY],
['tinyblob', 'tinyblob', MysqlAdapter::BLOB_TINY, MysqlAdapter::BLOB_TINY],
Comment on lines +1131 to +1132
Copy link
Member

@markstory markstory Oct 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How should we go about changing these types in the future? When we drop support for phinx in migrations, it would be beneficial to align with cakephp/database which uses type=binary and length parameters.

Copy link
Member Author

@dereuromark dereuromark Oct 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question
I added a test case for those as well now. You can check and if necessary adjust.

How it works:

The code in MysqlAdapter.php already:

  • Uses PHINX_TYPE_BINARY = TableSchemaInterface::TYPE_BINARY (line 50 in AdapterInterface.php)
  • Has blob constants aligned with CakePHP: BLOB_TINY = TableSchema::LENGTH_TINY, etc. (lines 72-76)
  • mapColumnData() handles both APIs and converts to CakePHP's type='binary' with length (lines 286-317)
  • CakePHP's MysqlSchemaDialect generates the correct SQL (TINYBLOB, MEDIUMBLOB, etc.) based on the length

Future work (when Phinx support is dropped):

You could:

  1. Deprecate Phinx-style blob types (tinyblob, mediumblob, longblob)
  2. Remove the round-trip conversion in mapColumnType() that converts back to blob types
  3. Keep only 'binary' with length parameter - fully aligned with CakePHP core
  4. This would be a breaking change requiring coordination with a major version bump

Copy link
Member Author

@dereuromark dereuromark Oct 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to revert, those didnt pass.
Feel free to add ammending commits to what you see fit. I just tried to get this ticket resolved.

['tinyblob', 'mediumblob', MysqlAdapter::BLOB_TINY + 20, MysqlAdapter::BLOB_MEDIUM],
['tinyblob', 'mediumblob', MysqlAdapter::BLOB_MEDIUM, MysqlAdapter::BLOB_MEDIUM],
['tinyblob', 'longblob', MysqlAdapter::BLOB_LONG, MysqlAdapter::BLOB_LONG],
// // Regular blobs
['blob', 'binary', MysqlAdapter::BLOB_TINY, MysqlAdapter::BLOB_TINY],
['blob', 'binary', null, null],
['blob', 'binary', MysqlAdapter::BLOB_REGULAR, null],
// Regular blobs
['blob', 'tinyblob', MysqlAdapter::BLOB_TINY, MysqlAdapter::BLOB_TINY],
['blob', 'blob', null, MysqlAdapter::BLOB_REGULAR],
['blob', 'blob', MysqlAdapter::BLOB_REGULAR, MysqlAdapter::BLOB_REGULAR],
['blob', 'mediumblob', MysqlAdapter::BLOB_MEDIUM, MysqlAdapter::BLOB_MEDIUM],
['blob', 'longblob', MysqlAdapter::BLOB_LONG, MysqlAdapter::BLOB_LONG],
// // medium blobs
['mediumblob', 'binary', MysqlAdapter::BLOB_TINY, MysqlAdapter::BLOB_TINY],
['mediumblob', 'binary', MysqlAdapter::BLOB_REGULAR, null],
// Medium blobs
['mediumblob', 'tinyblob', MysqlAdapter::BLOB_TINY, MysqlAdapter::BLOB_TINY],
['mediumblob', 'blob', MysqlAdapter::BLOB_REGULAR, MysqlAdapter::BLOB_REGULAR],
['mediumblob', 'mediumblob', null, MysqlAdapter::BLOB_MEDIUM],
['mediumblob', 'mediumblob', MysqlAdapter::BLOB_MEDIUM, MysqlAdapter::BLOB_MEDIUM],
['mediumblob', 'longblob', MysqlAdapter::BLOB_LONG, MysqlAdapter::BLOB_LONG],
// long blobs
['longblob', 'binary', MysqlAdapter::BLOB_TINY, MysqlAdapter::BLOB_TINY],
['longblob', 'binary', MysqlAdapter::BLOB_REGULAR, null],
// Long blobs
['longblob', 'tinyblob', MysqlAdapter::BLOB_TINY, MysqlAdapter::BLOB_TINY],
['longblob', 'blob', MysqlAdapter::BLOB_REGULAR, MysqlAdapter::BLOB_REGULAR],
['longblob', 'mediumblob', MysqlAdapter::BLOB_MEDIUM, MysqlAdapter::BLOB_MEDIUM],
['longblob', 'longblob', null, MysqlAdapter::BLOB_LONG],
['longblob', 'longblob', MysqlAdapter::BLOB_LONG, MysqlAdapter::BLOB_LONG],
Expand All @@ -1163,6 +1166,42 @@ public function testblobColumns(string $type, string $expectedType, ?int $limit,
$this->assertSame($expectedLimit, $columns[1]->getLimit());
}

public static function blobRoundTripData()
{
return [
// type, limit, expected type after round-trip, expected limit after round-trip
['blob', null, 'blob', MysqlAdapter::BLOB_REGULAR],
['blob', MysqlAdapter::BLOB_REGULAR, 'blob', MysqlAdapter::BLOB_REGULAR],
['tinyblob', null, 'tinyblob', MysqlAdapter::BLOB_TINY],
['mediumblob', null, 'mediumblob', MysqlAdapter::BLOB_MEDIUM],
['longblob', null, 'longblob', MysqlAdapter::BLOB_LONG],
];
}

#[DataProvider('blobRoundTripData')]
public function testBlobRoundTrip(string $type, ?int $limit, string $expectedType, int $expectedLimit)
{
// Create a table with a BLOB column
$table = new Table('blob_round_trip_test', [], $this->adapter);
$table->addColumn('blob_col', $type, ['limit' => $limit])
->save();

// Read the column back from the database
$columns = $this->adapter->getColumns('blob_round_trip_test');

$blobColumn = $columns[1];
$this->assertNotNull($blobColumn, 'BLOB column not found');
$this->assertSame($expectedType, $blobColumn->getType(), 'Type mismatch after round-trip');
$this->assertSame($expectedLimit, $blobColumn->getLimit(), 'Limit mismatch after round-trip');

// Verify that the SQL type is correct
$sqlType = $this->adapter->getSqlType($blobColumn->getType(), $blobColumn->getLimit());
$this->assertSame($type, $sqlType['name']);

// Clean up
$this->adapter->dropTable('blob_round_trip_test');
}

public function testBigIntegerColumn()
{
$table = new Table('t', [], $this->adapter);
Expand Down Expand Up @@ -1289,7 +1328,7 @@ public static function columnsProvider()
['column9', 'time', []],
['column10', 'timestamp', []],
['column11', 'date', []],
['column12', 'binary', []],
['column12', 'blob', []], // binary with no limit becomes BLOB in MySQL
['column13', 'boolean', ['comment' => 'Lorem ipsum']],
['column14', 'string', ['limit' => 10]],
['column16', 'geometry', []],
Expand Down
Loading