Commit d066c9fa authored by catch's avatar catch

Issue #2537928 by stefan.r, dawehner, alexpott, webflo, jhedstrom: MySQL index...

Issue #2537928 by stefan.r, dawehner, alexpott, webflo, jhedstrom: MySQL index normalization only works on table create
parent fe5ed1a7
......@@ -905,16 +905,22 @@ function db_drop_unique_key($table, $name) {
* The name of the index.
* @param array $fields
* An array of field names.
* @param array $spec
* The table specification of the table to be altered, as taken from a schema
* definition. See \Drupal\Core\Database\Schema::addIndex() for how to obtain
* this specification.
*
* @deprecated as of Drupal 8.0.x, will be removed in Drupal 9.0.0. Instead, get
* a database connection injected into your service from the container, get
* its schema driver, and call addIndex() on it. E.g.
* $injected_database->schema()->addIndex($table, $name, $fields);
* $injected_database->schema()->addIndex($table, $name, $fields, $spec);
*
* @see hook_schema()
* @see schemaapi
* @see \Drupal\Core\Database\Schema::addIndex()
*/
function db_add_index($table, $name, $fields) {
return Database::getConnection()->schema()->addIndex($table, $name, $fields);
function db_add_index($table, $name, $fields, array $spec) {
return Database::getConnection()->schema()->addIndex($table, $name, $fields, $spec);
}
/**
......
......@@ -9,6 +9,7 @@
use Drupal\Core\Database\Database;
use Drupal\Core\Database\Query\Condition;
use Drupal\Core\Database\SchemaException;
use Drupal\Core\Database\SchemaObjectExistsException;
use Drupal\Core\Database\SchemaObjectDoesNotExistException;
use Drupal\Core\Database\Schema as DatabaseSchema;
......@@ -299,14 +300,17 @@ protected function createKeysSql($spec) {
* Shortens indexes to 191 characters if they apply to utf8mb4-encoded
* fields, in order to comply with the InnoDB index limitation of 756 bytes.
*
* @param $spec
* @param array $spec
* The table specification.
*
* @return array
* List of shortened indexes.
*
* @throws \Drupal\Core\Database\SchemaException
* Thrown if field specification is missing.
*/
protected function getNormalizedIndexes($spec) {
$indexes = $spec['indexes'];
protected function getNormalizedIndexes(array $spec) {
$indexes = isset($spec['indexes']) ? $spec['indexes'] : [];
foreach ($indexes as $index_name => $index_fields) {
foreach ($index_fields as $index_key => $index_field) {
// Get the name of the field from the index specification.
......@@ -323,6 +327,9 @@ protected function getNormalizedIndexes($spec) {
}
}
}
else {
throw new SchemaException("MySQL needs the '$field_name' field specification in order to normalize the '$index_name' index");
}
}
}
return $indexes;
......@@ -486,7 +493,10 @@ public function dropUniqueKey($table, $name) {
return TRUE;
}
public function addIndex($table, $name, $fields) {
/**
* {@inheritdoc}
*/
public function addIndex($table, $name, $fields, array $spec) {
if (!$this->tableExists($table)) {
throw new SchemaObjectDoesNotExistException(t("Cannot add index @name to table @table: table doesn't exist.", array('@table' => $table, '@name' => $name)));
}
......@@ -494,7 +504,10 @@ public function addIndex($table, $name, $fields) {
throw new SchemaObjectExistsException(t("Cannot add index @name to table @table: index already exists.", array('@table' => $table, '@name' => $name)));
}
$this->connection->query('ALTER TABLE {' . $table . '} ADD INDEX `' . $name . '` (' . $this->createKeySql($fields) . ')');
$spec['indexes'][$name] = $fields;
$indexes = $this->getNormalizedIndexes($spec);
$this->connection->query('ALTER TABLE {' . $table . '} ADD INDEX `' . $name . '` (' . $this->createKeySql($indexes[$name]) . ')');
}
public function dropIndex($table, $name) {
......
......@@ -637,7 +637,10 @@ public function dropUniqueKey($table, $name) {
return TRUE;
}
public function addIndex($table, $name, $fields) {
/**
* {@inheritdoc}
*/
public function addIndex($table, $name, $fields, array $spec) {
if (!$this->tableExists($table)) {
throw new SchemaObjectDoesNotExistException(t("Cannot add index @name to table @table: table doesn't exist.", array('@table' => $table, '@name' => $name)));
}
......@@ -779,7 +782,10 @@ protected function _createKeys($table, $new_keys) {
}
if (isset($new_keys['indexes'])) {
foreach ($new_keys['indexes'] as $name => $fields) {
$this->addIndex($table, $name, $fields);
// Even though $new_keys is not a full schema it still has 'indexes' and
// so is a partial schema. Technically addIndex() doesn't do anything
// with it so passing an empty array would work as well.
$this->addIndex($table, $name, $fields, $new_keys);
}
}
}
......
......@@ -582,7 +582,10 @@ protected function mapKeyDefinition(array $key_definition, array $mapping) {
return $key_definition;
}
public function addIndex($table, $name, $fields) {
/**
* {@inheritdoc}
*/
public function addIndex($table, $name, $fields, array $spec) {
if (!$this->tableExists($table)) {
throw new SchemaObjectDoesNotExistException(t("Cannot add index @name to table @table: table doesn't exist.", array('@table' => $table, '@name' => $name)));
}
......
......@@ -413,13 +413,51 @@ public function fieldExists($table, $column) {
* @code
* $fields = ['foo', ['bar', 4]];
* @endcode
* @param array $spec
* The table specification for the table to be altered. This is used in
* order to be able to ensure that the index length is not too long.
* This schema definition can usually be obtained through hook_schema(), or
* in case the table was created by the Entity API, through the schema
* handler listed in the entity class definition. For reference, see
* SqlContentEntityStorageSchema::getDedicatedTableSchema() and
* SqlContentEntityStorageSchema::getSharedTableFieldSchema().
*
* In order to prevent human error, it is recommended to pass in the
* complete table specification. However, in the edge case of the complete
* table specification not being available, we can pass in a partial table
* definition containing only the fields that apply to the index:
* @code
* $spec = [
* // Example partial specification for a table:
* 'fields' => [
* 'example_field' => [
* 'description' => 'An example field',
* 'type' => 'varchar',
* 'length' => 32,
* 'not null' => TRUE,
* 'default' => '',
* ],
* ],
* 'indexes' => [
* 'table_example_field' => ['example_field'],
* ],
* ];
* @endcode
* Note that the above is a partial table definition and that we would
* usually pass a complete table definition as obtained through
* hook_schema() instead.
*
* @see schemaapi
* @see hook_schema()
*
* @throws \Drupal\Core\Database\SchemaObjectDoesNotExistException
* If the specified table doesn't exist.
* @throws \Drupal\Core\Database\SchemaObjectExistsException
* If the specified table already has an index by that name.
*
* @todo remove the $spec argument whenever schema introspection is added.
*/
abstract public function addIndex($table, $name, $fields);
abstract public function addIndex($table, $name, $fields, array $spec);
/**
* Drop an index.
......
......@@ -330,9 +330,13 @@ public function onEntityTypeUpdate(EntityTypeInterface $entity_type, EntityTypeI
// Create new indexes and unique keys.
$entity_schema = $this->getEntitySchema($entity_type, TRUE);
foreach ($this->getEntitySchemaData($entity_type, $entity_schema) as $table_name => $schema) {
// Add fields schema because database driver may depend on this data to
// perform index normalization.
$schema['fields'] = $entity_schema[$table_name]['fields'];
if (!empty($schema['indexes'])) {
foreach ($schema['indexes'] as $name => $specifier) {
$schema_handler->addIndex($table_name, $name, $specifier);
$schema_handler->addIndex($table_name, $name, $specifier, $schema);
}
}
if (!empty($schema['unique keys'])) {
......@@ -1139,7 +1143,7 @@ protected function createSharedTableSchema(FieldStorageDefinitionInterface $stor
// Check if the index exists because it might already have been
// created as part of the earlier entity type update event.
if (!$schema_handler->indexExists($table_name, $name)) {
$schema_handler->addIndex($table_name, $name, $specifier);
$schema_handler->addIndex($table_name, $name, $specifier, $schema[$table_name]);
}
}
}
......@@ -1273,9 +1277,13 @@ protected function updateDedicatedTableSchema(FieldStorageDefinitionInterface $s
$table = $table_mapping->getDedicatedDataTableName($original);
$revision_table = $table_mapping->getDedicatedRevisionTableName($original);
// Get the field schemas.
$schema = $storage_definition->getSchema();
$original_schema = $original->getSchema();
// Gets the SQL schema for a dedicated tables.
$actual_schema = $this->getDedicatedTableSchema($storage_definition);
foreach ($original_schema['indexes'] as $name => $columns) {
if (!isset($schema['indexes'][$name]) || $columns != $schema['indexes'][$name]) {
$real_name = $this->getFieldIndexName($storage_definition, $name);
......@@ -1302,8 +1310,8 @@ protected function updateDedicatedTableSchema(FieldStorageDefinitionInterface $s
$real_columns[] = $table_mapping->getFieldColumnName($storage_definition, $column_name);
}
}
$this->database->schema()->addIndex($table, $real_name, $real_columns);
$this->database->schema()->addIndex($revision_table, $real_name, $real_columns);
$this->database->schema()->addIndex($table, $real_name, $real_columns, $actual_schema[$table]);
$this->database->schema()->addIndex($revision_table, $real_name, $real_columns, $actual_schema[$revision_table]);
}
}
$this->saveFieldSchemaData($storage_definition, $this->getDedicatedTableSchema($storage_definition));
......@@ -1380,7 +1388,7 @@ protected function updateSharedTableSchema(FieldStorageDefinitionInterface $stor
// Create new indexes and unique keys.
if (!empty($schema[$table_name]['indexes'])) {
foreach ($schema[$table_name]['indexes'] as $name => $specifier) {
$schema_handler->addIndex($table_name, $name, $specifier);
$schema_handler->addIndex($table_name, $name, $specifier, $schema[$table_name]);
}
}
if (!empty($schema[$table_name]['unique keys'])) {
......
......@@ -8,6 +8,7 @@
namespace Drupal\system\Tests\Database;
use Drupal\Core\Database\Database;
use Drupal\Core\Database\SchemaException;
use Drupal\Core\Database\SchemaObjectDoesNotExistException;
use Drupal\Core\Database\SchemaObjectExistsException;
use Drupal\simpletest\KernelTestBase;
......@@ -99,7 +100,7 @@ function testSchema() {
$index_exists = Database::getConnection()->schema()->indexExists('test_table', 'test_field');
$this->assertIdentical($index_exists, FALSE, 'Fake index does not exists');
// Add index.
db_add_index('test_table', 'test_field', array('test_field'));
db_add_index('test_table', 'test_field', array('test_field'), $table_specification);
// Test for created index and test for the boolean result of indexExists().
$index_exists = Database::getConnection()->schema()->indexExists('test_table', 'test_field');
$this->assertIdentical($index_exists, TRUE, 'Index created.');
......@@ -295,6 +296,43 @@ function testIndexLength() {
);
db_create_table('test_table_index_length', $table_specification);
$schema_object = Database::getConnection()->schema();
// Ensure expected exception thrown when adding index with missing info.
$expected_exception_message = "MySQL needs the 'test_field_text' field specification in order to normalize the 'test_regular' index";
$missing_field_spec = $table_specification;
unset($missing_field_spec['fields']['test_field_text']);
try {
$schema_object->addIndex('test_table_index_length', 'test_separate', [['test_field_text', 200]], $missing_field_spec);
$this->fail('SchemaException not thrown when adding index with missing information.');
}
catch (SchemaException $e) {
$this->assertEqual($expected_exception_message, $e->getMessage());
}
// Add a separate index.
$schema_object->addIndex('test_table_index_length', 'test_separate', [['test_field_text', 200]], $table_specification);
$table_specification_with_new_index = $table_specification;
$table_specification_with_new_index['indexes']['test_separate'] = [['test_field_text', 200]];
// Ensure that the exceptions of addIndex are thrown as expected.
try {
$schema_object->addIndex('test_table_index_length', 'test_separate', [['test_field_text', 200]], $table_specification);
$this->fail('\Drupal\Core\Database\SchemaObjectExistsException exception missed.');
}
catch (SchemaObjectExistsException $e) {
$this->pass('\Drupal\Core\Database\SchemaObjectExistsException thrown when index already exists.');
}
try {
$schema_object->addIndex('test_table_non_existing', 'test_separate', [['test_field_text', 200]], $table_specification);
$this->fail('\Drupal\Core\Database\SchemaObjectDoesNotExistException exception missed.');
}
catch (SchemaObjectDoesNotExistException $e) {
$this->pass('\Drupal\Core\Database\SchemaObjectDoesNotExistException thrown when index already exists.');
}
// Get index information.
$results = db_query('SHOW INDEX FROM {test_table_index_length}');
$expected_lengths = array(
......@@ -316,11 +354,14 @@ function testIndexLength() {
'test_field_string_ascii_long' => 200,
'test_field_string_short' => NULL,
),
'test_separate' => array(
'test_field_text' => 191,
),
);
// Count the number of columns defined in the indexes.
$column_count = 0;
foreach ($table_specification['indexes'] as $index) {
foreach ($table_specification_with_new_index['indexes'] as $index) {
foreach ($index as $field) {
$column_count++;
}
......
......@@ -7,6 +7,7 @@
namespace Drupal\system\Tests\Entity;
use Drupal\Core\Database\Database;
use Drupal\Core\Database\DatabaseExceptionWrapper;
use Drupal\Core\Database\IntegrityConstraintViolationException;
use Drupal\Core\Entity\EntityDefinitionUpdateManagerInterface;
......@@ -626,4 +627,48 @@ public function testSingleActionCalls() {
$this->assertTrue($this->database->schema()->tableExists('entity_test_update_revision'), "The 'entity_test_update_revision' table has been created.");
}
/**
* Ensures that a new field and index on a shared table are created.
*
* @see Drupal\Core\Entity\Sql\SqlContentEntityStorageSchema::createSharedTableSchema
*/
public function testCreateFieldAndIndexOnSharedTable() {
$this->addBaseField();
$this->addBaseFieldIndex();
$this->entityDefinitionUpdateManager->applyUpdates();
$this->assertTrue($this->database->schema()->fieldExists('entity_test_update', 'new_base_field'), "New field 'new_base_field' has been created on the 'entity_test_update' table.");
$this->assertTrue($this->database->schema()->indexExists('entity_test_update', 'entity_test_update_field__new_base_field'), "New index 'entity_test_update_field__new_base_field' has been created on the 'entity_test_update' table.");
// Check index size in for MySQL.
if (Database::getConnection()->driver() == 'mysql') {
$result = Database::getConnection()->query('SHOW INDEX FROM {entity_test_update} WHERE key_name = \'entity_test_update_field__new_base_field\' and column_name = \'new_base_field\'')->fetchObject();
$this->assertEqual(191, $result->Sub_part, 'The index length has been restricted to 191 characters for UTF8MB4 compatibility.');
}
}
/**
* Ensures that a new entity level index is created when data exists.
*
* @see Drupal\Core\Entity\Sql\SqlContentEntityStorageSchema::onEntityTypeUpdate
*/
public function testCreateIndexUsingEntityStorageSchemaWithData() {
// Save an entity.
$name = $this->randomString();
$storage = $this->entityManager->getStorage('entity_test_update');
$entity = $storage->create(array('name' => $name));
$entity->save();
// Create an index.
$indexes = array(
'entity_test_update__type_index' => array('type'),
);
$this->state->set('entity_test_update.additional_entity_indexes', $indexes);
$this->entityDefinitionUpdateManager->applyUpdates();
$this->assertTrue($this->database->schema()->indexExists('entity_test_update', 'entity_test_update__type_index'), "New index 'entity_test_update__type_index' has been created on the 'entity_test_update' table.");
// Check index size in for MySQL.
if (Database::getConnection()->driver() == 'mysql') {
$result = Database::getConnection()->query('SHOW INDEX FROM {entity_test_update} WHERE key_name = \'entity_test_update__type_index\' and column_name = \'type\'')->fetchObject();
$this->assertEqual(191, $result->Sub_part, 'The index length has been restricted to 191 characters for UTF8MB4 compatibility.');
}
}
}
......@@ -35,7 +35,14 @@ function update_test_schema_schema() {
* Schema version 8001.
*/
function update_test_schema_update_8001() {
$table = [
'fields' => [
'a' => ['type' => 'int', 'not null' => TRUE],
'b' => ['type' => 'blob', 'not null' => FALSE],
],
];
// Add a column.
db_add_index('update_test_schema_table', 'test', ['a']);
db_add_index('update_test_schema_table', 'test', ['a'], $table);
}
}
......@@ -18,6 +18,13 @@
*/
class SqlContentEntityStorageSchemaTest extends UnitTestCase {
/**
* The mocked DB schema handler.
*
* @var \Drupal\Core\Database\Schema|\PHPUnit_Framework_MockObject_MockObject
*/
protected $dbSchemaHandler;
/**
* The mocked entity manager used in this test.
*
......@@ -1298,7 +1305,7 @@ protected function setUpStorageSchema(array $expected = array()) {
->with($this->entityType->id())
->will($this->returnValue($this->storageDefinitions));
$db_schema_handler = $this->getMockBuilder('Drupal\Core\Database\Schema')
$this->dbSchemaHandler = $this->getMockBuilder('Drupal\Core\Database\Schema')
->disableOriginalConstructor()
->getMock();
......@@ -1307,7 +1314,7 @@ protected function setUpStorageSchema(array $expected = array()) {
$expected_table_names = array_keys($expected);
$expected_table_schemas = array_values($expected);
$db_schema_handler->expects($this->any())
$this->dbSchemaHandler->expects($this->any())
->method('createTable')
->with(
$this->callback(function($table_name) use (&$invocation_count, $expected_table_names) {
......@@ -1327,17 +1334,21 @@ protected function setUpStorageSchema(array $expected = array()) {
->getMock();
$connection->expects($this->any())
->method('schema')
->will($this->returnValue($db_schema_handler));
->will($this->returnValue($this->dbSchemaHandler));
$key_value = $this->getMock('Drupal\Core\KeyValueStore\KeyValueStoreInterface');
$this->storageSchema = $this->getMockBuilder('Drupal\Core\Entity\Sql\SqlContentEntityStorageSchema')
->setConstructorArgs(array($this->entityManager, $this->entityType, $this->storage, $connection))
->setMethods(array('installedStorageSchema', 'loadEntitySchemaData', 'hasSharedTableNameChanges'))
->setMethods(array('installedStorageSchema', 'loadEntitySchemaData', 'hasSharedTableNameChanges', 'isTableEmpty'))
->getMock();
$this->storageSchema
->expects($this->any())
->method('installedStorageSchema')
->will($this->returnValue($key_value));
$this->storageSchema
->expects($this->any())
->method('isTableEmpty')
->willReturn(FALSE);
}
/**
......@@ -1380,4 +1391,76 @@ public function setUpStorageDefinition($field_name, array $schema) {
}
}
/**
* ::onEntityTypeUpdate
*/
public function testonEntityTypeUpdateWithNewIndex() {
$entity_type_id = 'entity_test';
$this->entityType = $original_entity_type = new ContentEntityType(array(
'id' => 'entity_test',
'entity_keys' => array('id' => 'id'),
));
// Add a field with a really long index.
$this->setUpStorageDefinition('long_index_name', array(
'columns' => array(
'long_index_name' => array(
'type' => 'int',
),
),
'indexes' => array(
'long_index_name_really_long_long_name' => array(array('long_index_name', 10)),
),
));
$expected = array(
'entity_test' => array(
'description' => 'The base table for entity_test entities.',
'fields' => array(
'id' => array(
'type' => 'serial',
'not null' => TRUE,
),
'long_index_name' => array(
'type' => 'int',
'not null' => FALSE,
),
),
'indexes' => array(
'entity_test__b588603cb9' => array(
array('long_index_name', 10),
),
),
),
);
$this->setUpStorageSchema($expected);
$table_mapping = new DefaultTableMapping($this->entityType, $this->storageDefinitions);
$table_mapping->setFieldNames('entity_test', array_keys($this->storageDefinitions));
$table_mapping->setExtraColumns('entity_test', array('default_langcode'));
$this->storage->expects($this->any())
->method('getTableMapping')
->will($this->returnValue($table_mapping));
$this->storageSchema->expects($this->any())
->method('loadEntitySchemaData')
->willReturn([]);
$this->dbSchemaHandler->expects($this->atLeastOnce())
->method('addIndex')
->with('entity_test', 'entity_test__b588603cb9', [['long_index_name', 10]], $this->callback(function($actual_value) use ($expected) {
$this->assertEquals($expected['entity_test']['indexes'], $actual_value['indexes']);
$this->assertEquals($expected['entity_test']['fields'], $actual_value['fields']);
// If the parameters don't match, the assertions above will throw an
// exception.
return TRUE;
}));
$this->assertNull(
$this->storageSchema->onEntityTypeUpdate($this->entityType, $original_entity_type)
);
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment