diff --git a/core/lib/Drupal/Core/Database/Driver/mysql/Schema.php b/core/lib/Drupal/Core/Database/Driver/mysql/Schema.php index 9ecc89a2ca63f14ceecd576791ec78fe984f110f..48a3bb20e6a41dd9e5b76b5a926909fae56c4e52 100644 --- a/core/lib/Drupal/Core/Database/Driver/mysql/Schema.php +++ b/core/lib/Drupal/Core/Database/Driver/mysql/Schema.php @@ -460,6 +460,18 @@ public function dropField($table, $field) { return FALSE; } + // When dropping a field that is part of a composite primary key MySQL + // automatically removes the field from the primary key, which can leave the + // table in an invalid state. MariaDB 10.2.8 requires explicitly dropping + // the primary key first for this reason. We perform this deletion + // explicitly which also makes the behavior on both MySQL and MariaDB + // consistent with PostgreSQL. + // @see https://mariadb.com/kb/en/library/alter-table + $primary_key = $this->findPrimaryKeyColumns($table); + if ((count($primary_key) > 1) && in_array($field, $primary_key, TRUE)) { + $this->dropPrimaryKey($table); + } + $this->connection->query('ALTER TABLE {' . $table . '} DROP `' . $field . '`'); return TRUE; } diff --git a/core/lib/Drupal/Core/Database/Driver/sqlite/Schema.php b/core/lib/Drupal/Core/Database/Driver/sqlite/Schema.php index 168297d3ac88123731d4f9b10d9e3dbb8285355a..ad2ab2d5b7d45f1a71de2aa03f840a63c6f1926b 100644 --- a/core/lib/Drupal/Core/Database/Driver/sqlite/Schema.php +++ b/core/lib/Drupal/Core/Database/Driver/sqlite/Schema.php @@ -551,9 +551,11 @@ public function dropField($table, $field) { unset($new_schema['fields'][$field]); - // Handle possible primary key changes. - if (isset($new_schema['primary key']) && ($key = array_search($field, $new_schema['primary key'])) !== FALSE) { - unset($new_schema['primary key'][$key]); + // Drop the primary key if the field to drop is part of it. This is + // consistent with the behavior on PostgreSQL. + // @see \Drupal\Core\Database\Driver\mysql\Schema::dropField() + if (isset($new_schema['primary key']) && in_array($field, $new_schema['primary key'], TRUE)) { + unset($new_schema['primary key']); } // Handle possible index changes. @@ -633,8 +635,10 @@ protected function mapKeyDefinition(array $key_definition, array $mapping) { if (is_array($field)) { $field = &$field[0]; } - if (isset($mapping[$field])) { - $field = $mapping[$field]; + + $mapped_field = array_search($field, $mapping, TRUE); + if ($mapped_field !== FALSE) { + $field = $mapped_field; } } return $key_definition; diff --git a/core/tests/Drupal/KernelTests/Core/Database/SchemaTest.php b/core/tests/Drupal/KernelTests/Core/Database/SchemaTest.php index 278d75635ddc4a99381611fc88332a430296b78f..d638996cc8f2d880bb1edb24766fca9485e50dc8 100644 --- a/core/tests/Drupal/KernelTests/Core/Database/SchemaTest.php +++ b/core/tests/Drupal/KernelTests/Core/Database/SchemaTest.php @@ -498,9 +498,9 @@ public function tryUnsignedInsert($table_name, $column_name) { } /** - * Tests adding columns to an existing table. + * Tests adding columns to an existing table with default and initial value. */ - public function testSchemaAddField() { + public function testSchemaAddFieldDefaultInitial() { // Test varchar types. foreach ([1, 32, 128, 256, 512] as $length) { $base_field_spec = [ @@ -704,9 +704,119 @@ protected function assertFieldCharacteristics($table_name, $field_name, $field_s } /** - * Tests changing columns between types. + * Tests various schema changes' effect on the table's primary key. + * + * @param array $initial_primary_key + * The initial primary key of the test table. + * @param array $renamed_primary_key + * The primary key of the test table after renaming the test field. + * + * @dataProvider providerTestSchemaCreateTablePrimaryKey + * + * @covers ::addField + * @covers ::changeField + * @covers ::dropField + * @covers ::findPrimaryKeyColumns + */ + public function testSchemaChangePrimaryKey(array $initial_primary_key, array $renamed_primary_key) { + $connection = Database::getConnection(); + $schema = $connection->schema(); + $find_primary_key_columns = new \ReflectionMethod(get_class($schema), 'findPrimaryKeyColumns'); + $find_primary_key_columns->setAccessible(TRUE); + + // Test making the field the primary key of the table upon creation. + $table_name = 'test_table'; + $table_spec = [ + 'fields' => [ + 'test_field' => ['type' => 'int'], + 'other_test_field' => ['type' => 'int'], + ], + 'primary key' => $initial_primary_key, + ]; + $schema->createTable($table_name, $table_spec); + $this->assertTrue($schema->fieldExists($table_name, 'test_field')); + $this->assertEquals($initial_primary_key, $find_primary_key_columns->invoke($schema, $table_name)); + + // Change the field type and make sure the primary key stays in place. + $schema->changeField($table_name, 'test_field', 'test_field', ['type' => 'varchar', 'length' => 32]); + $this->assertTrue($schema->fieldExists($table_name, 'test_field')); + $this->assertEquals($initial_primary_key, $find_primary_key_columns->invoke($schema, $table_name)); + + // Add some data and change the field type back, to make sure that changing + // the type leaves the primary key in place even with existing data. + $connection->insert($table_name) + ->fields(['test_field' => 1, 'other_test_field' => 2]) + ->execute(); + $schema->changeField($table_name, 'test_field', 'test_field', ['type' => 'int']); + $this->assertTrue($schema->fieldExists($table_name, 'test_field')); + $this->assertEquals($initial_primary_key, $find_primary_key_columns->invoke($schema, $table_name)); + + // Make sure that adding the primary key can be done as part of changing + // a field, as well. + $schema->dropPrimaryKey($table_name); + $this->assertEquals([], $find_primary_key_columns->invoke($schema, $table_name)); + $schema->changeField($table_name, 'test_field', 'test_field', ['type' => 'int'], ['primary key' => $initial_primary_key]); + $this->assertTrue($schema->fieldExists($table_name, 'test_field')); + $this->assertEquals($initial_primary_key, $find_primary_key_columns->invoke($schema, $table_name)); + + // Rename the field and make sure the primary key was updated. + $schema->changeField($table_name, 'test_field', 'test_field_renamed', ['type' => 'int']); + $this->assertTrue($schema->fieldExists($table_name, 'test_field_renamed')); + $this->assertEquals($renamed_primary_key, $find_primary_key_columns->invoke($schema, $table_name)); + + // Drop the field and make sure the primary key was dropped, as well. + $schema->dropField($table_name, 'test_field_renamed'); + $this->assertFalse($schema->fieldExists($table_name, 'test_field_renamed')); + $this->assertEquals([], $find_primary_key_columns->invoke($schema, $table_name)); + + // Add the field again and make sure adding the primary key can be done at + // the same time. + $schema->addField($table_name, 'test_field', ['type' => 'int', 'default' => 0], ['primary key' => $initial_primary_key]); + $this->assertTrue($schema->fieldExists($table_name, 'test_field')); + $this->assertEquals($initial_primary_key, $find_primary_key_columns->invoke($schema, $table_name)); + + // Drop the field again and explicitly add a primary key. + $schema->dropField($table_name, 'test_field'); + $schema->addPrimaryKey($table_name, ['other_test_field']); + $this->assertFalse($schema->fieldExists($table_name, 'test_field')); + $this->assertEquals(['other_test_field'], $find_primary_key_columns->invoke($schema, $table_name)); + + // Test that adding a field with a primary key will work even with a + // pre-existing primary key. + $schema->addField($table_name, 'test_field', ['type' => 'int', 'default' => 0], ['primary key' => $initial_primary_key]); + $this->assertTrue($schema->fieldExists($table_name, 'test_field')); + $this->assertEquals($initial_primary_key, $find_primary_key_columns->invoke($schema, $table_name)); + } + + /** + * Provides test cases for SchemaTest::testSchemaCreateTablePrimaryKey(). + * + * @return array + * An array of test cases for SchemaTest::testSchemaCreateTablePrimaryKey(). + */ + public function providerTestSchemaCreateTablePrimaryKey() { + $tests = []; + + $tests['simple_primary_key'] = [ + 'initial_primary_key' => ['test_field'], + 'renamed_primary_key' => ['test_field_renamed'], + ]; + $tests['composite_primary_key'] = [ + 'initial_primary_key' => ['test_field', 'other_test_field'], + 'renamed_primary_key' => ['test_field_renamed', 'other_test_field'], + ]; + $tests['composite_primary_key_different_order'] = [ + 'initial_primary_key' => ['other_test_field', 'test_field'], + 'renamed_primary_key' => ['other_test_field', 'test_field_renamed'], + ]; + + return $tests; + } + + /** + * Tests changing columns between types with default and initial values. */ - public function testSchemaChangeField() { + public function testSchemaChangeFieldDefaultInitial() { $field_specs = [ ['type' => 'int', 'size' => 'normal', 'not null' => FALSE], ['type' => 'int', 'size' => 'normal', 'not null' => TRUE, 'initial' => 1, 'default' => 17],