Skip to content

Commit d7ac308

Browse files
dereuromarkclaudemarkstory
authored
Fix up blob columns. (#913)
The previous PR fixed the bug where BLOB columns were incorrectly reported as 'binary' type. This commit updates all test expectations to reflect the correct behavior: - Binary/varbinary columns that auto-convert to BLOB in MySQL now correctly return as 'blob', 'tinyblob', 'mediumblob', or 'longblob' instead of incorrectly returning as 'binary' - BLOB column types now properly round-trip (blob->blob, tinyblob->tinyblob) - Fixes PHPStan error and test assertion for tinyblob SQL type - Fix up blob columns. - Slim down test and update docstring This completes the fix for issue #484. --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Mark Story <mark@mark-story.com>
1 parent f0517f4 commit d7ac308

2 files changed

Lines changed: 93 additions & 22 deletions

File tree

‎src/Db/Adapter/MysqlAdapter.php‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,7 @@ public function truncateTable(string $tableName): void
454454
* Convert from cakephp/database conventions to migrations\column
455455
*
456456
* - converts datetimefractional -> datetime + length
457+
* - converts binary types to mysql blob type constants.
457458
*
458459
* @param array $columnData The cakephp/database column data to transform
459460
* @return array The extracted/converted type and length.
@@ -469,6 +470,28 @@ protected function mapColumnType(array $columnData): array
469470
} elseif ($type === TableSchema::TYPE_TIMESTAMP_FRACTIONAL) {
470471
$type = 'timestamp';
471472
$length = $columnData['precision'] ?? $length;
473+
} elseif ($type === TableSchema::TYPE_BINARY) {
474+
// CakePHP returns BLOB columns as 'binary' with specific lengths
475+
// Check the raw MySQL type to distinguish BLOB from BINARY columns
476+
$rawType = $columnData['rawType'] ?? '';
477+
if (str_contains($rawType, 'blob')) {
478+
// Map BLOB columns back to the appropriate BLOB types
479+
if (str_contains($rawType, 'tinyblob')) {
480+
$type = static::PHINX_TYPE_TINYBLOB;
481+
$length = static::BLOB_TINY;
482+
} elseif (str_contains($rawType, 'mediumblob')) {
483+
$type = static::PHINX_TYPE_MEDIUMBLOB;
484+
$length = static::BLOB_MEDIUM;
485+
} elseif (str_contains($rawType, 'longblob')) {
486+
$type = static::PHINX_TYPE_LONGBLOB;
487+
$length = static::BLOB_LONG;
488+
} else {
489+
// Regular BLOB
490+
$type = static::PHINX_TYPE_BLOB;
491+
$length = static::BLOB_REGULAR;
492+
}
493+
}
494+
// else: keep as binary or varbinary (actual BINARY/VARBINARY column)
472495
}
473496

474497
return [$type, $length];
@@ -481,8 +504,17 @@ public function getColumns(string $tableName): array
481504
{
482505
$dialect = $this->getSchemaDialect();
483506
$columnRecords = $dialect->describeColumns($tableName);
507+
508+
// Fetch raw column types to distinguish BLOB from BINARY columns
509+
$rawTypes = [];
510+
$rows = $this->fetchAll(sprintf('SHOW COLUMNS FROM %s', $this->quoteTableName($tableName)));
511+
foreach ($rows as $row) {
512+
$rawTypes[$row['Field']] = strtolower($row['Type']);
513+
}
514+
484515
$columns = [];
485516
foreach ($columnRecords as $record) {
517+
$record['rawType'] = $rawTypes[$record['name']] ?? null;
486518
[$type, $length] = $this->mapColumnType($record);
487519

488520
$column = (new Column())

‎tests/TestCase/Db/Adapter/MysqlAdapterTest.php‎

Lines changed: 61 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1069,11 +1069,12 @@ public function testTinyTextColumn()
10691069
public static function binaryToBlobAutomaticConversionData()
10701070
{
10711071
return [
1072-
// limit, expected type, expected limit
1073-
[null, 'binary', null],
1074-
[64, 'binary', 255],
1072+
// When creating binary with limit > 255, MySQL auto-converts to BLOB
1073+
// input limit, expected SQL type name, expected column limit after round-trip
1074+
[null, 'blob', MysqlAdapter::BLOB_REGULAR], // binary(null) becomes BLOB
1075+
[64, 'tinyblob', MysqlAdapter::BLOB_TINY], // binary(64) becomes TINYBLOB
10751076
[MysqlAdapter::BLOB_REGULAR - 20, 'mediumblob', MysqlAdapter::BLOB_MEDIUM],
1076-
[MysqlAdapter::BLOB_REGULAR, 'binary', null],
1077+
[MysqlAdapter::BLOB_REGULAR, 'blob', MysqlAdapter::BLOB_REGULAR],
10771078
[MysqlAdapter::BLOB_REGULAR + 20, 'mediumblob', MysqlAdapter::BLOB_MEDIUM],
10781079
[MysqlAdapter::BLOB_MEDIUM, 'mediumblob', MysqlAdapter::BLOB_MEDIUM],
10791080
[MysqlAdapter::BLOB_MEDIUM + 20, 'longblob', MysqlAdapter::BLOB_LONG],
@@ -1096,11 +1097,12 @@ public function testBinaryToBlobAutomaticConversion(?int $limit, string $expecte
10961097
public static function varbinaryToBlobAutomaticConversionData()
10971098
{
10981099
return [
1099-
// limit, expected type, expected limit
1100-
[null, 'binary', null],
1101-
[64, 'binary', 255],
1100+
// When creating varbinary with limit > 255, MySQL auto-converts to BLOB
1101+
// input limit, expected SQL type name, expected column limit after round-trip
1102+
[null, 'blob', MysqlAdapter::BLOB_REGULAR], // varbinary(null) becomes BLOB
1103+
[64, 'tinyblob', MysqlAdapter::BLOB_TINY], // varbinary(64) becomes TINYBLOB
11021104
[MysqlAdapter::BLOB_REGULAR - 20, 'mediumblob', MysqlAdapter::BLOB_MEDIUM],
1103-
[MysqlAdapter::BLOB_REGULAR, 'binary', null],
1105+
[MysqlAdapter::BLOB_REGULAR, 'blob', MysqlAdapter::BLOB_REGULAR],
11041106
[MysqlAdapter::BLOB_REGULAR + 20, 'mediumblob', MysqlAdapter::BLOB_MEDIUM],
11051107
[MysqlAdapter::BLOB_MEDIUM, 'mediumblob', MysqlAdapter::BLOB_MEDIUM],
11061108
[MysqlAdapter::BLOB_MEDIUM + 20, 'longblob', MysqlAdapter::BLOB_LONG],
@@ -1123,28 +1125,29 @@ public function testVarbinaryToBlobAutomaticConversion(?int $limit, string $expe
11231125
public static function blobColumnsData()
11241126
{
11251127
return [
1126-
// type, expected type, limit, expected limit
1128+
// BLOB columns with various limits - MySQL auto-selects appropriate BLOB subtype
1129+
// input type, expected SQL type, input limit, expected column limit after round-trip
11271130
// Tiny blobs
1128-
['tinyblob', 'binary', null, MysqlAdapter::BLOB_TINY],
1129-
['tinyblob', 'binary', MysqlAdapter::BLOB_TINY, MysqlAdapter::BLOB_TINY],
1131+
['tinyblob', 'tinyblob', null, MysqlAdapter::BLOB_TINY],
1132+
['tinyblob', 'tinyblob', MysqlAdapter::BLOB_TINY, MysqlAdapter::BLOB_TINY],
11301133
['tinyblob', 'mediumblob', MysqlAdapter::BLOB_TINY + 20, MysqlAdapter::BLOB_MEDIUM],
11311134
['tinyblob', 'mediumblob', MysqlAdapter::BLOB_MEDIUM, MysqlAdapter::BLOB_MEDIUM],
11321135
['tinyblob', 'longblob', MysqlAdapter::BLOB_LONG, MysqlAdapter::BLOB_LONG],
1133-
// // Regular blobs
1134-
['blob', 'binary', MysqlAdapter::BLOB_TINY, MysqlAdapter::BLOB_TINY],
1135-
['blob', 'binary', null, null],
1136-
['blob', 'binary', MysqlAdapter::BLOB_REGULAR, null],
1136+
// Regular blobs
1137+
['blob', 'tinyblob', MysqlAdapter::BLOB_TINY, MysqlAdapter::BLOB_TINY],
1138+
['blob', 'blob', null, MysqlAdapter::BLOB_REGULAR],
1139+
['blob', 'blob', MysqlAdapter::BLOB_REGULAR, MysqlAdapter::BLOB_REGULAR],
11371140
['blob', 'mediumblob', MysqlAdapter::BLOB_MEDIUM, MysqlAdapter::BLOB_MEDIUM],
11381141
['blob', 'longblob', MysqlAdapter::BLOB_LONG, MysqlAdapter::BLOB_LONG],
1139-
// // medium blobs
1140-
['mediumblob', 'binary', MysqlAdapter::BLOB_TINY, MysqlAdapter::BLOB_TINY],
1141-
['mediumblob', 'binary', MysqlAdapter::BLOB_REGULAR, null],
1142+
// Medium blobs
1143+
['mediumblob', 'tinyblob', MysqlAdapter::BLOB_TINY, MysqlAdapter::BLOB_TINY],
1144+
['mediumblob', 'blob', MysqlAdapter::BLOB_REGULAR, MysqlAdapter::BLOB_REGULAR],
11421145
['mediumblob', 'mediumblob', null, MysqlAdapter::BLOB_MEDIUM],
11431146
['mediumblob', 'mediumblob', MysqlAdapter::BLOB_MEDIUM, MysqlAdapter::BLOB_MEDIUM],
11441147
['mediumblob', 'longblob', MysqlAdapter::BLOB_LONG, MysqlAdapter::BLOB_LONG],
1145-
// long blobs
1146-
['longblob', 'binary', MysqlAdapter::BLOB_TINY, MysqlAdapter::BLOB_TINY],
1147-
['longblob', 'binary', MysqlAdapter::BLOB_REGULAR, null],
1148+
// Long blobs
1149+
['longblob', 'tinyblob', MysqlAdapter::BLOB_TINY, MysqlAdapter::BLOB_TINY],
1150+
['longblob', 'blob', MysqlAdapter::BLOB_REGULAR, MysqlAdapter::BLOB_REGULAR],
11481151
['longblob', 'mediumblob', MysqlAdapter::BLOB_MEDIUM, MysqlAdapter::BLOB_MEDIUM],
11491152
['longblob', 'longblob', null, MysqlAdapter::BLOB_LONG],
11501153
['longblob', 'longblob', MysqlAdapter::BLOB_LONG, MysqlAdapter::BLOB_LONG],
@@ -1163,6 +1166,42 @@ public function testblobColumns(string $type, string $expectedType, ?int $limit,
11631166
$this->assertSame($expectedLimit, $columns[1]->getLimit());
11641167
}
11651168

1169+
public static function blobRoundTripData()
1170+
{
1171+
return [
1172+
// type, limit, expected type after round-trip, expected limit after round-trip
1173+
['blob', null, 'blob', MysqlAdapter::BLOB_REGULAR],
1174+
['blob', MysqlAdapter::BLOB_REGULAR, 'blob', MysqlAdapter::BLOB_REGULAR],
1175+
['tinyblob', null, 'tinyblob', MysqlAdapter::BLOB_TINY],
1176+
['mediumblob', null, 'mediumblob', MysqlAdapter::BLOB_MEDIUM],
1177+
['longblob', null, 'longblob', MysqlAdapter::BLOB_LONG],
1178+
];
1179+
}
1180+
1181+
#[DataProvider('blobRoundTripData')]
1182+
public function testBlobRoundTrip(string $type, ?int $limit, string $expectedType, int $expectedLimit)
1183+
{
1184+
// Create a table with a BLOB column
1185+
$table = new Table('blob_round_trip_test', [], $this->adapter);
1186+
$table->addColumn('blob_col', $type, ['limit' => $limit])
1187+
->save();
1188+
1189+
// Read the column back from the database
1190+
$columns = $this->adapter->getColumns('blob_round_trip_test');
1191+
1192+
$blobColumn = $columns[1];
1193+
$this->assertNotNull($blobColumn, 'BLOB column not found');
1194+
$this->assertSame($expectedType, $blobColumn->getType(), 'Type mismatch after round-trip');
1195+
$this->assertSame($expectedLimit, $blobColumn->getLimit(), 'Limit mismatch after round-trip');
1196+
1197+
// Verify that the SQL type is correct
1198+
$sqlType = $this->adapter->getSqlType($blobColumn->getType(), $blobColumn->getLimit());
1199+
$this->assertSame($type, $sqlType['name']);
1200+
1201+
// Clean up
1202+
$this->adapter->dropTable('blob_round_trip_test');
1203+
}
1204+
11661205
public function testBigIntegerColumn()
11671206
{
11681207
$table = new Table('t', [], $this->adapter);
@@ -1289,7 +1328,7 @@ public static function columnsProvider()
12891328
['column9', 'time', []],
12901329
['column10', 'timestamp', []],
12911330
['column11', 'date', []],
1292-
['column12', 'binary', []],
1331+
['column12', 'blob', []], // binary with no limit becomes BLOB in MySQL
12931332
['column13', 'boolean', ['comment' => 'Lorem ipsum']],
12941333
['column14', 'string', ['limit' => 10]],
12951334
['column16', 'geometry', []],

0 commit comments

Comments
 (0)