Commit 72cd9735 authored by catch's avatar catch
Browse files

Issue #2616724 by amateescu, daffie: Warn when trying to create a database...

Issue #2616724 by amateescu, daffie: Warn when trying to create a database table with a NOT NULL => FALSE primary key
parent 4b1a8df0
......@@ -108,6 +108,9 @@ protected function createTableSql($name, $table) {
}
// Process keys & indexes.
if (!empty($table['primary key']) && is_array($table['primary key'])) {
$this->ensureNotNullPrimaryKey($table['primary key'], $table['fields']);
}
$keys = $this->createKeysSql($table);
if (count($keys)) {
$sql .= implode(", \n", $keys) . ", \n";
......@@ -409,6 +412,9 @@ public function addField($table, $field, $spec, $keys_new = []) {
// Fields that are part of a PRIMARY KEY must be added as NOT NULL.
$is_primary_key = isset($keys_new['primary key']) && in_array($field, $keys_new['primary key'], TRUE);
if ($is_primary_key) {
$this->ensureNotNullPrimaryKey($keys_new['primary key'], [$field => $spec]);
}
$fixnull = FALSE;
if (!empty($spec['not null']) && !isset($spec['default']) && !$is_primary_key) {
......@@ -610,6 +616,9 @@ public function changeField($table, $field, $field_new, $spec, $keys_new = []) {
if (($field != $field_new) && $this->fieldExists($table, $field_new)) {
throw new SchemaObjectExistsException(t("Cannot rename field @table.@name to @name_new: target field already exists.", ['@table' => $table, '@name' => $field, '@name_new' => $field_new]));
}
if (isset($keys_new['primary key']) && in_array($field_new, $keys_new['primary key'], TRUE)) {
$this->ensureNotNullPrimaryKey($keys_new['primary key'], [$field_new => $spec]);
}
$sql = 'ALTER TABLE {' . $table . '} CHANGE `' . $field . '` ' . $this->createFieldSql($field_new, $this->processField($spec));
if ($keys_sql = $this->createKeysSql($keys_new)) {
......
......@@ -250,6 +250,7 @@ protected function createTableSql($name, $table) {
$sql_keys = [];
if (!empty($table['primary key']) && is_array($table['primary key'])) {
$this->ensureNotNullPrimaryKey($table['primary key'], $table['fields']);
$sql_keys[] = 'CONSTRAINT ' . $this->ensureIdentifiersLength($name, '', 'pkey') . ' PRIMARY KEY (' . $this->createPrimaryKeySql($table['primary key']) . ')';
}
if (isset($table['unique keys']) && is_array($table['unique keys'])) {
......@@ -551,7 +552,10 @@ public function addField($table, $field, $spec, $new_keys = []) {
}
// Fields that are part of a PRIMARY KEY must be added as NOT NULL.
$is_primary_key = isset($keys_new['primary key']) && in_array($field, $keys_new['primary key'], TRUE);
$is_primary_key = isset($new_keys['primary key']) && in_array($field, $new_keys['primary key'], TRUE);
if ($is_primary_key) {
$this->ensureNotNullPrimaryKey($new_keys['primary key'], [$field => $spec]);
}
$fixnull = FALSE;
if (!empty($spec['not null']) && !isset($spec['default']) && !$is_primary_key) {
......@@ -804,6 +808,9 @@ public function changeField($table, $field, $field_new, $spec, $new_keys = []) {
if (($field != $field_new) && $this->fieldExists($table, $field_new)) {
throw new SchemaObjectExistsException(t("Cannot rename field @table.@name to @name_new: target field already exists.", ['@table' => $table, '@name' => $field, '@name_new' => $field_new]));
}
if (isset($new_keys['primary key']) && in_array($field_new, $new_keys['primary key'], TRUE)) {
$this->ensureNotNullPrimaryKey($new_keys['primary key'], [$field_new => $spec]);
}
$spec = $this->processField($spec);
......
......@@ -52,6 +52,10 @@ public function fieldExists($table, $column) {
* An array of SQL statements to create the table.
*/
public function createTableSql($name, $table) {
if (!empty($table['primary key']) && is_array($table['primary key'])) {
$this->ensureNotNullPrimaryKey($table['primary key'], $table['fields']);
}
$sql = [];
$sql[] = "CREATE TABLE {" . $name . "} (\n" . $this->createColumnsSql($name, $table) . "\n)\n";
return array_merge($sql, $this->createIndexSql($name, $table));
......@@ -315,6 +319,9 @@ public function addField($table, $field, $specification, $keys_new = []) {
if ($this->fieldExists($table, $field)) {
throw new SchemaObjectExistsException(t("Cannot add field @table.@field: field already exists.", ['@field' => $field, '@table' => $table]));
}
if (isset($keys_new['primary key']) && in_array($field, $keys_new['primary key'], TRUE)) {
$this->ensureNotNullPrimaryKey($keys_new['primary key'], [$field => $specification]);
}
// SQLite doesn't have a full-featured ALTER TABLE statement. It only
// supports adding new fields to a table, in some simple cases. In most
......@@ -495,7 +502,7 @@ protected function introspectSchema($table) {
$schema['fields'][$row->name] = [
'type' => $type,
'size' => $size,
'not null' => !empty($row->notnull),
'not null' => !empty($row->notnull) || $row->pk !== "0",
'default' => trim($row->dflt_value, "'"),
];
if ($length) {
......@@ -584,6 +591,9 @@ public function changeField($table, $field, $field_new, $spec, $keys_new = []) {
if (($field != $field_new) && $this->fieldExists($table, $field_new)) {
throw new SchemaObjectExistsException(t("Cannot rename field @table.@name to @name_new: target field already exists.", ['@table' => $table, '@name' => $field, '@name_new' => $field_new]));
}
if (isset($keys_new['primary key']) && in_array($field_new, $keys_new['primary key'], TRUE)) {
$this->ensureNotNullPrimaryKey($keys_new['primary key'], [$field_new => $spec]);
}
$old_schema = $this->introspectSchema($table);
$new_schema = $old_schema;
......@@ -733,6 +743,7 @@ public function addPrimaryKey($table, $fields) {
}
$new_schema['primary key'] = $fields;
$this->ensureNotNullPrimaryKey($new_schema['primary key'], $new_schema['fields']);
$this->alterTable($table, $old_schema, $new_schema);
}
......
......@@ -682,4 +682,25 @@ protected function escapeDefaultValue($value) {
return is_string($value) ? $this->connection->quote($value) : $value;
}
/**
* Ensures that all the primary key fields are correctly defined.
*
* @param array $primary_key
* An array containing the fields that will form the primary key of a table.
* @param array $fields
* An array containing the field specifications of the table, as per the
* schema data structure format.
*
* @throws \Drupal\Core\Database\SchemaException
* Thrown if any primary key field specification does not exist or if they
* do not define 'not null' as TRUE.
*/
protected function ensureNotNullPrimaryKey(array $primary_key, array $fields) {
foreach (array_intersect($primary_key, array_keys($fields)) as $field_name) {
if (!isset($fields[$field_name]['not null']) || $fields[$field_name]['not null'] !== TRUE) {
throw new SchemaException("The '$field_name' field specification does not define 'not null' as TRUE.");
}
}
}
}
......@@ -728,8 +728,8 @@ public function testSchemaChangePrimaryKey(array $initial_primary_key, array $re
$table_name = 'test_table';
$table_spec = [
'fields' => [
'test_field' => ['type' => 'int'],
'other_test_field' => ['type' => 'int'],
'test_field' => ['type' => 'int', 'not null' => TRUE],
'other_test_field' => ['type' => 'int', 'not null' => TRUE],
],
'primary key' => $initial_primary_key,
];
......@@ -738,7 +738,7 @@ public function testSchemaChangePrimaryKey(array $initial_primary_key, array $re
$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]);
$schema->changeField($table_name, 'test_field', 'test_field', ['type' => 'varchar', 'length' => 32, 'not null' => TRUE]);
$this->assertTrue($schema->fieldExists($table_name, 'test_field'));
$this->assertEquals($initial_primary_key, $find_primary_key_columns->invoke($schema, $table_name));
......@@ -747,7 +747,7 @@ public function testSchemaChangePrimaryKey(array $initial_primary_key, array $re
$connection->insert($table_name)
->fields(['test_field' => 1, 'other_test_field' => 2])
->execute();
$schema->changeField($table_name, 'test_field', 'test_field', ['type' => 'int']);
$schema->changeField($table_name, 'test_field', 'test_field', ['type' => 'int', 'not null' => TRUE]);
$this->assertTrue($schema->fieldExists($table_name, 'test_field'));
$this->assertEquals($initial_primary_key, $find_primary_key_columns->invoke($schema, $table_name));
......@@ -755,12 +755,12 @@ public function testSchemaChangePrimaryKey(array $initial_primary_key, array $re
// 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]);
$schema->changeField($table_name, 'test_field', 'test_field', ['type' => 'int', 'not null' => TRUE], ['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']);
$schema->changeField($table_name, 'test_field', 'test_field_renamed', ['type' => 'int', 'not null' => TRUE]);
$this->assertTrue($schema->fieldExists($table_name, 'test_field_renamed'));
$this->assertEquals($renamed_primary_key, $find_primary_key_columns->invoke($schema, $table_name));
......@@ -771,7 +771,7 @@ public function testSchemaChangePrimaryKey(array $initial_primary_key, array $re
// 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]);
$schema->addField($table_name, 'test_field', ['type' => 'int', 'default' => 0, 'not null' => TRUE], ['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));
......@@ -783,7 +783,7 @@ public function testSchemaChangePrimaryKey(array $initial_primary_key, array $re
// 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]);
$schema->addField($table_name, 'test_field', ['type' => 'int', 'default' => 0, 'not null' => TRUE], ['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));
}
......@@ -813,6 +813,71 @@ public function providerTestSchemaCreateTablePrimaryKey() {
return $tests;
}
/**
* Tests an invalid field specification as a primary key on table creation.
*/
public function testInvalidPrimaryKeyOnTableCreation() {
$connection = Database::getConnection();
$schema = $connection->schema();
// Test making an invalid field the primary key of the table upon creation.
$table_name = 'test_table';
$table_spec = [
'fields' => [
'test_field' => ['type' => 'int'],
],
'primary key' => ['test_field'],
];
$this->expectException(SchemaException::class);
$this->expectExceptionMessage("The 'test_field' field specification does not define 'not null' as TRUE.");
$schema->createTable($table_name, $table_spec);
}
/**
* Tests adding an invalid field specification as a primary key.
*/
public function testInvalidPrimaryKeyAddition() {
$connection = Database::getConnection();
$schema = $connection->schema();
// Test adding a new invalid field to the primary key.
$table_name = 'test_table';
$table_spec = [
'fields' => [
'test_field' => ['type' => 'int', 'not null' => TRUE],
],
'primary key' => ['test_field'],
];
$schema->createTable($table_name, $table_spec);
$this->expectException(SchemaException::class);
$this->expectExceptionMessage("The 'new_test_field' field specification does not define 'not null' as TRUE.");
$schema->addField($table_name, 'new_test_field', ['type' => 'int'], ['primary key' => ['test_field', 'new_test_field']]);
}
/**
* Tests changing the primary key with an invalid field specification.
*/
public function testInvalidPrimaryKeyChange() {
$connection = Database::getConnection();
$schema = $connection->schema();
// Test adding a new invalid field to the primary key.
$table_name = 'test_table';
$table_spec = [
'fields' => [
'test_field' => ['type' => 'int', 'not null' => TRUE],
],
'primary key' => ['test_field'],
];
$schema->createTable($table_name, $table_spec);
$this->expectException(SchemaException::class);
$this->expectExceptionMessage("The 'changed_test_field' field specification does not define 'not null' as TRUE.");
$schema->dropPrimaryKey($table_name);
$schema->changeField($table_name, 'test_field', 'changed_test_field', ['type' => 'int'], ['primary key' => ['changed_test_field']]);
}
/**
* Tests changing columns between types with default and initial values.
*/
......
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