Skip to content

Commit

Permalink
Merge pull request #9749 from gurucomkz/patch-9453-mysql8-ints
Browse files Browse the repository at this point in the history
FIX Don't use int width for mysql >= 8.0.17 #9453
  • Loading branch information
Maxime Rainville authored Dec 12, 2021
2 parents 97ab957 + d5de29f commit 27d7c2f
Show file tree
Hide file tree
Showing 4 changed files with 266 additions and 5 deletions.
11 changes: 11 additions & 0 deletions docs/en/00_Getting_Started/00_Server_Requirements.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,17 @@ setting. It is generally recommended to leave this setting as-is because it resu
some advanced cases, the sql_mode can be configured on the database connection via the configuration API (
see `MySQLDatabase::$sql_mode` for more details.) This setting is only available in Silverstripe CMS 4.7 and later.

### MySQL/MariaDB Int width in schema

MySQL 8.0.17 stopped reporting the width attribute for integers while MariaDB did not change its behaviour.
This results in constant rebuilding of the schema when MySQLSchemaManager expects a field to look like e.g.
`INT(8)` and MySQL server reports it simply as `INT`. MySQLSchemaManager has been updated to detect the MySQL
server implementation and act accordingly. In cases when auto-detection fails, you can force the desired behaviour like this:
```yml
SilverStripe\ORM\Connect\MySQLSchemaManager:
schema_use_int_width: true # or false when INT widths should be ignored
```
## Webserver Configuration
### Overview
Expand Down
30 changes: 25 additions & 5 deletions src/ORM/Connect/MySQLSchemaManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,23 @@ public function renameField($tableName, $oldName, $newName)

protected static $_cache_collation_info = [];

private function shouldUseIntegerWidth()
{
// MySQL 8.0.17 stopped reporting the width attribute for integers
// https://github.com/silverstripe/silverstripe-framework/issues/9453
// Note: MariaDB did not change its behaviour
$forceWidth = Config::inst()->get(self::class, 'schema_use_int_width');
if ($forceWidth !== null) {
return $forceWidth;
}
$v = $this->database->getVersion();
if (false !== strpos($v, 'MariaDB')) {
// MariaDB is included in the version string: https://mariadb.com/kb/en/version/
return true;
}
return version_compare($v, '8.0.17', '<');
}

public function fieldList($table)
{
$fields = $this->query("SHOW FULL FIELDS IN \"$table\"");
Expand Down Expand Up @@ -405,7 +422,8 @@ public function boolean($values)
//'default'=>$this->default);
//DB::requireField($this->tableName, $this->name, "tinyint(1) unsigned not null default
//'{$this->defaultVal}'");
return 'tinyint(1) unsigned not null' . $this->defaultClause($values);
$width = $this->shouldUseIntegerWidth() ? '(1)' : '';
return 'tinyint' . $width . ' unsigned not null' . $this->defaultClause($values);
}

/**
Expand Down Expand Up @@ -518,7 +536,8 @@ public function int($values)
//For reference, this is what typically gets passed to this function:
//$parts=Array('datatype'=>'int', 'precision'=>11, 'null'=>'not null', 'default'=>(int)$this->default);
//DB::requireField($this->tableName, $this->name, "int(11) not null default '{$this->defaultVal}'");
return "int(11) not null" . $this->defaultClause($values);
$width = $this->shouldUseIntegerWidth() ? '(11)' : '';
return 'int' . $width . ' not null' . $this->defaultClause($values);
}

/**
Expand All @@ -534,8 +553,8 @@ public function bigint($values)
// 'arrayValue'=>$this->arrayValue);
//$values=Array('type'=>'bigint', 'parts'=>$parts);
//DB::requireField($this->tableName, $this->name, $values);

return 'bigint(20) not null' . $this->defaultClause($values);
$width = $this->shouldUseIntegerWidth() ? '(20)' : '';
return 'bigint' . $width . ' not null' . $this->defaultClause($values);
}

/**
Expand Down Expand Up @@ -616,7 +635,8 @@ public function year($values)

public function IdColumn($asDbValue = false, $hasAutoIncPK = true)
{
return 'int(11) not null auto_increment';
$width = $this->shouldUseIntegerWidth() ? '(11)' : '';
return 'int' . $width . ' not null auto_increment';
}

/**
Expand Down
209 changes: 209 additions & 0 deletions tests/php/ORM/MySQLSchemaManagerTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,209 @@
<?php

namespace SilverStripe\ORM\Tests;

use SilverStripe\Core\Config\Config;
use SilverStripe\ORM\Connect\MySQLSchemaManager;
use SilverStripe\Dev\SapphireTest;
use SilverStripe\ORM\Tests\MySQLSchemaManagerTest\MySQLDBDummy;

class MySQLSchemaManagerTest extends SapphireTest
{
public function testMYSQL_8_0_16()
{
Config::forClass(MySQLSchemaManager::class)->set('schema_use_int_width', null);

$mysqlDBdummy = new MySQLDBDummy('8.0.16-standard');
$mgr = new MySQLSchemaManager();
$mgr->setDatabase($mysqlDBdummy);

$this->assertEquals(
'tinyint(1) unsigned not null',
$mgr->boolean([]),
'mysql 8.0.16 boolean has width'
);

$this->assertEquals(
'int(11) not null',
$mgr->int([]),
'mysql 8.0.16 int has width'
);

$this->assertEquals(
'bigint(20) not null',
$mgr->bigint([]),
'mysql 8.0.16 bigint has width'
);

$this->assertEquals(
'int(11) not null auto_increment',
$mgr->IdColumn([]),
'mysql 8.0.16 IdColumn has width'
);
}

public function testMYSQL_8_0_17()
{
Config::forClass(MySQLSchemaManager::class)->set('schema_use_int_width', null);

$mysqlDBdummy = new MySQLDBDummy('8.0.17');
$mgr = new MySQLSchemaManager();
$mgr->setDatabase($mysqlDBdummy);

$this->assertEquals(
'tinyint unsigned not null',
$mgr->boolean([]),
'mysql 8.0.17 boolean has no width'
);

$this->assertEquals(
'int not null',
$mgr->int([]),
'mysql 8.0.17 int has no width'
);

$this->assertEquals(
'bigint not null',
$mgr->bigint([]),
'mysql 8.0.17 bigint has no width'
);

$this->assertEquals(
'int not null auto_increment',
$mgr->IdColumn([]),
'mysql 8.0.17 IdColumn has no width'
);
}

public function testMariaDB()
{
Config::forClass(MySQLSchemaManager::class)->set('schema_use_int_width', null);

$mariaDBdummy = new MySQLDBDummy('10.4.7-MariaDB');
$mgr = new MySQLSchemaManager();
$mgr->setDatabase($mariaDBdummy);

$this->assertEquals(
'tinyint(1) unsigned not null',
$mgr->boolean([]),
'mariadb boolean has width'
);

$this->assertEquals(
'int(11) not null',
$mgr->int([]),
'mariadb int has width'
);

$this->assertEquals(
'bigint(20) not null',
$mgr->bigint([]),
'mariadb bigint has width'
);

$this->assertEquals(
'int(11) not null auto_increment',
$mgr->IdColumn([]),
'mariadb IdColumn has width'
);
}

public function testMySQLForcedON()
{
Config::forClass(MySQLSchemaManager::class)->set('schema_use_int_width', true);

$mysqlDBdummy = new MySQLDBDummy('8.0.17-standard');
$mgr = new MySQLSchemaManager();
$mgr->setDatabase($mysqlDBdummy);

$this->assertEquals(
'tinyint(1) unsigned not null',
$mgr->boolean([]),
'mysql 8.0.17 boolean forced on has width'
);

$this->assertEquals(
'int(11) not null',
$mgr->int([]),
'mysql 8.0.17 int forced on has width'
);

$this->assertEquals(
'bigint(20) not null',
$mgr->bigint([]),
'mysql 8.0.17 bigint forced on has width'
);

$this->assertEquals(
'int(11) not null auto_increment',
$mgr->IdColumn([]),
'mysql 8.0.17 IdColumn forced on has width'
);
}

public function testMySQLForcedOFF()
{
Config::forClass(MySQLSchemaManager::class)->set('schema_use_int_width', false);

$mysqlDBdummy = new MySQLDBDummy('8.0.16-standard');
$mgr = new MySQLSchemaManager();
$mgr->setDatabase($mysqlDBdummy);

$this->assertEquals(
'tinyint unsigned not null',
$mgr->boolean([]),
'mysql 8.0.16 boolean forced off has no width'
);

$this->assertEquals(
'int not null',
$mgr->int([]),
'mysql 8.0.16 int forced off has no width'
);

$this->assertEquals(
'bigint not null',
$mgr->bigint([]),
'mysql 8.0.16 bigint forced off has no width'
);

$this->assertEquals(
'int not null auto_increment',
$mgr->IdColumn([]),
'mysql 8.0.16 IdColumn forced off has no width'
);
}

public function testMariaDBForcedOFF()
{
Config::forClass(MySQLSchemaManager::class)->set('schema_use_int_width', false);

$mysqlDBdummy = new MySQLDBDummy('10.0.1-MariaDB');
$mgr = new MySQLSchemaManager();
$mgr->setDatabase($mysqlDBdummy);

$this->assertEquals(
'tinyint unsigned not null',
$mgr->boolean([]),
'mariadb boolean forced off has no width'
);

$this->assertEquals(
'int not null',
$mgr->int([]),
'mariadb int forced off has no width'
);

$this->assertEquals(
'bigint not null',
$mgr->bigint([]),
'mariadb bigint forced off has no width'
);

$this->assertEquals(
'int not null auto_increment',
$mgr->IdColumn([]),
'mariadb IdColumn forced off has no width'
);
}
}
21 changes: 21 additions & 0 deletions tests/php/ORM/MySQLSchemaManagerTest/MySQLDBDummy.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?php

namespace SilverStripe\ORM\Tests\MySQLSchemaManagerTest;

use SilverStripe\Dev\TestOnly;
use SilverStripe\ORM\Connect\MySQLDatabase;

class MySQLDBDummy extends MySQLDatabase implements TestOnly
{
private $dbVersion;

public function __construct($version)
{
$this->dbVersion = $version;
}

public function getVersion()
{
return $this->dbVersion;
}
}

0 comments on commit 27d7c2f

Please sign in to comment.