Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Fix SQL Server default constraints #298

Merged
merged 1 commit into from

3 participants

@deeky666
Collaborator

This PR fixes altering column default values. In SQL Server column default values are stored in constraints. CREATE TABLE statements with column declarations like some_column NVARCHAR(50) NOT NULL DEFAULT 'default value' internally creates a default constraint with an automatically generated name in the the system table sys.default_constraints. ALTER TABLE statements do not support the DEFAULT clause in column alteration declarations, leading in SQL syntax errors. Thus changing a column's default value is currently not possible.
To alter a column's default value, the old column's default constraint hast to be dropped and recreated again. As a default constraint has to be referenced by name to be dropped, we need to create each default constraint with an own unique name. This PR generates separate statements for default constraint declarations. It generates a unique name consisting of the table name and the column name the default constraint is created for.
DF_TABLE_NAME_HASH%_%COLUMN_NAME_HASH%

@doctrinebot
Collaborator

Hello,

thank you for positing this Pull Request. I have automatically opened an issue on our Jira Bug Tracker for you with the details of this Pull-Request. See the Link:

http://doctrine-project.org/jira/browse/DBAL-484

@beberlei beberlei merged commit fe04d1f into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 4, 2013
  1. @deeky666
This page is out of date. Refresh to see the latest.
View
136 lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php
@@ -194,12 +194,22 @@ public function getDropIndexSQL($index, $table = null)
*/
protected function _getCreateTableSQL($tableName, array $columns, array $options = array())
{
+ $defaultConstraintsSql = array();
+
// @todo does other code breaks because of this?
// force primary keys to be not null
foreach ($columns as &$column) {
if (isset($column['primary']) && $column['primary']) {
$column['notnull'] = true;
}
+
+ /**
+ * Build default constraints SQL statements
+ */
+ if ( ! empty($column['default']) || is_numeric($column['default'])) {
+ $defaultConstraintsSql[] = 'ALTER TABLE ' . $tableName .
+ ' ADD' . $this->getDefaultConstraintDeclarationSQL($tableName, $column);
+ }
}
$columnListSql = $this->getColumnDeclarationListSQL($columns);
@@ -240,7 +250,7 @@ protected function _getCreateTableSQL($tableName, array $columns, array $options
}
}
- return $sql;
+ return array_merge($sql, $defaultConstraintsSql);
}
/**
@@ -256,6 +266,29 @@ public function getCreatePrimaryKeySQL(Index $index, $table)
}
/**
+ * Returns the SQL snippet for declaring a default constraint.
+ *
+ * @param string $table Name of the table to return the default constraint declaration for.
+ * @param array $column Column definition.
+ *
+ * @return string
+ *
+ * @throws \InvalidArgumentException
+ */
+ public function getDefaultConstraintDeclarationSQL($table, array $column)
+ {
+ if (empty($column['default']) && ! is_numeric($column['default'])) {
+ throw new \InvalidArgumentException("Incomplete column definition. 'default' required.");
+ }
+
+ return
+ ' CONSTRAINT ' .
+ $this->generateDefaultConstraintName($table, $column['name']) .
+ $this->getDefaultValueDeclarationSQL($column) .
+ ' FOR ' . $column['name'];
+ }
+
+ /**
* {@inheritDoc}
*/
public function getUniqueConstraintDeclarationSQL($name, Index $index)
@@ -331,12 +364,19 @@ public function getAlterTableSQL(TableDiff $diff)
$sql = array();
$columnSql = array();
+ /** @var \Doctrine\DBAL\Schema\Column $column */
foreach ($diff->addedColumns as $column) {
if ($this->onSchemaAlterTableAddColumn($column, $diff, $columnSql)) {
continue;
}
- $queryParts[] = 'ADD ' . $this->getColumnDeclarationSQL($column->getQuotedName($this), $column->toArray());
+ $columnDef = $column->toArray();
+ $queryParts[] = 'ADD ' . $this->getColumnDeclarationSQL($column->getQuotedName($this), $columnDef);
+
+ if ( ! empty($columnDef['default']) || is_numeric($columnDef['default'])) {
+ $columnDef['name'] = $column->getQuotedName($this);
+ $queryParts[] = 'ADD' . $this->getDefaultConstraintDeclarationSQL($diff->name, $columnDef);
+ }
}
foreach ($diff->removedColumns as $column) {
@@ -347,15 +387,35 @@ public function getAlterTableSQL(TableDiff $diff)
$queryParts[] = 'DROP COLUMN ' . $column->getQuotedName($this);
}
+ /* @var $columnDiff \Doctrine\DBAL\Schema\ColumnDiff */
foreach ($diff->changedColumns as $columnDiff) {
if ($this->onSchemaAlterTableChangeColumn($columnDiff, $diff, $columnSql)) {
continue;
}
- /* @var $columnDiff \Doctrine\DBAL\Schema\ColumnDiff */
+ $fromColumn = $columnDiff->fromColumn;
+ $fromColumnDefault = isset($fromColumn) ? $fromColumn->getDefault() : null;
$column = $columnDiff->column;
+ $columnDef = $column->toArray();
+ $columnDefaultHasChanged = $columnDiff->hasChanged('default');
+
+ /**
+ * Drop existing column default constraint
+ * if default value has changed and another
+ * default constraint already exists for the column.
+ */
+ if ($columnDefaultHasChanged && ( ! empty($fromColumnDefault) || is_numeric($fromColumnDefault))) {
+ $queryParts[] = 'DROP CONSTRAINT ' .
+ $this->generateDefaultConstraintName($diff->name, $columnDiff->oldColumnName);
+ }
+
$queryParts[] = 'ALTER COLUMN ' .
- $this->getColumnDeclarationSQL($column->getQuotedName($this), $column->toArray());
+ $this->getColumnDeclarationSQL($column->getQuotedName($this), $columnDef);
+
+ if ($columnDefaultHasChanged && (! empty($columnDef['default']) || is_numeric($columnDef['default']))) {
+ $columnDef['name'] = $column->getQuotedName($this);
+ $queryParts[] = 'ADD' . $this->getDefaultConstraintDeclarationSQL($diff->name, $columnDef);
+ }
}
foreach ($diff->renamedColumns as $oldColumnName => $column) {
@@ -364,8 +424,28 @@ public function getAlterTableSQL(TableDiff $diff)
}
$sql[] = "sp_RENAME '". $diff->name. ".". $oldColumnName . "' , '".$column->getQuotedName($this)."', 'COLUMN'";
+
+ $columnDef = $column->toArray();
+
+ /**
+ * Drop existing default constraint for the old column name
+ * if column has default value.
+ */
+ if ( ! empty($columnDef['default']) || is_numeric($columnDef['default'])) {
+ $queryParts[] = 'DROP CONSTRAINT ' .
+ $this->generateDefaultConstraintName($diff->name, $oldColumnName);
+ }
+
$queryParts[] = 'ALTER COLUMN ' .
- $this->getColumnDeclarationSQL($column->getQuotedName($this), $column->toArray());
+ $this->getColumnDeclarationSQL($column->getQuotedName($this), $columnDef);
+
+ /**
+ * Readd default constraint for the new column name.
+ */
+ if ( ! empty($columnDef['default']) || is_numeric($columnDef['default'])) {
+ $columnDef['name'] = $column->getQuotedName($this);
+ $queryParts[] = 'ADD' . $this->getDefaultConstraintDeclarationSQL($diff->name, $columnDef);
+ }
}
$tableSql = array();
@@ -382,6 +462,23 @@ public function getAlterTableSQL(TableDiff $diff)
if ($diff->newName !== false) {
$sql[] = "sp_RENAME '" . $diff->name . "', '" . $diff->newName . "'";
+
+ /**
+ * Rename table's default constraints names
+ * to match the new table name.
+ * This is necessary to ensure that the default
+ * constraints can be referenced in future table
+ * alterations as the table name is encoded in
+ * default constraints' names.
+ */
+ $sql[] = "DECLARE @sql NVARCHAR(MAX) = N''; " .
+ "SELECT @sql += N'EXEC sp_rename N''' + dc.name + ''', N''' " .
+ "+ REPLACE(dc.name, '" . $this->generateIdentifierName($diff->name) . "', " .
+ "'" . $this->generateIdentifierName($diff->newName) . "') + ''', ''OBJECT'';' " .
+ "FROM sys.default_constraints dc " .
+ "JOIN sys.tables tbl ON dc.parent_object_id = tbl.object_id " .
+ "WHERE tbl.name = '" . $diff->newName . "';" .
+ "EXEC sp_executesql @sql";
}
return array_merge($sql, $tableSql, $columnSql);
@@ -994,8 +1091,6 @@ public function getColumnDeclarationSQL($name, array $field)
if (isset($field['columnDefinition'])) {
$columnDef = $this->getCustomTypeDeclarationSQL($field);
} else {
- $default = $this->getDefaultValueDeclarationSQL($field);
-
$collation = (isset($field['collate']) && $field['collate']) ?
' ' . $this->getColumnCollationDeclarationSQL($field['collate']) : '';
@@ -1008,9 +1103,34 @@ public function getColumnDeclarationSQL($name, array $field)
' ' . $field['check'] : '';
$typeDecl = $field['type']->getSqlDeclaration($field, $this);
- $columnDef = $typeDecl . $collation . $default . $notnull . $unique . $check;
+ $columnDef = $typeDecl . $collation . $notnull . $unique . $check;
}
return $name . ' ' . $columnDef;
}
+
+ /**
+ * Returns a unique default constraint name for a table and column.
+ *
+ * @param string $table Name of the table to generate the unique default constraint name for.
+ * @param string $column Name of the column in the table to generate the unique default constraint name for.
+ *
+ * @return string
+ */
+ private function generateDefaultConstraintName($table, $column)
+ {
+ return 'DF_' . $this->generateIdentifierName($table) . '_' . $this->generateIdentifierName($column);
+ }
+
+ /**
+ * Returns a hash value for a given identifier.
+ *
+ * @param string $identifier Identifier to generate a hash value for.
+ *
+ * @return string
+ */
+ private function generateIdentifierName($identifier)
+ {
+ return strtoupper(dechex(crc32($identifier)));
+ }
}
View
4 lib/Doctrine/DBAL/Schema/SQLServerSchemaManager.php
@@ -60,6 +60,10 @@ protected function _getPortableTableColumnDefinition($tableColumn)
while ($default != ($default2 = preg_replace("/^\((.*)\)$/", '$1', $default))) {
$default = trim($default2, "'");
+
+ if ($default == 'getdate()') {
+ $default = $this->_platform->getCurrentTimestampSQL();
+ }
}
switch ($dbType) {
View
112 tests/Doctrine/Tests/DBAL/Functional/Schema/SQLServerSchemaManagerTest.php
@@ -52,4 +52,116 @@ public function testCollationCharset()
$this->assertEquals($collation, $columns[$columnName]->getPlatformOption('collate'));
}
+
+ public function testDefaultContraints()
+ {
+ $table = new Table('sqlsrv_df_constraints');
+ $table->addColumn('no_default', 'string');
+ $table->addColumn('df_integer', 'integer', array('default' => 666));
+ $table->addColumn('df_string_1', 'string', array('default' => 'foobar'));
+ $table->addColumn('df_string_2', 'string', array('default' => 'Doctrine rocks!!!'));
+ $table->addColumn('df_string_3', 'string', array('default' => 'another default value'));
+ $table->addColumn('df_string_4', 'string', array('default' => 'column to rename'));
+ $table->addColumn('df_boolean', 'boolean', array('default' => true));
+
+ $this->_sm->createTable($table);
+ $columns = $this->_sm->listTableColumns('sqlsrv_df_constraints');
+
+ $this->assertNull($columns['no_default']->getDefault());
+ $this->assertEquals(666, $columns['df_integer']->getDefault());
+ $this->assertEquals('foobar', $columns['df_string_1']->getDefault());
+ $this->assertEquals('Doctrine rocks!!!', $columns['df_string_2']->getDefault());
+ $this->assertEquals('another default value', $columns['df_string_3']->getDefault());
+ $this->assertEquals(1, $columns['df_boolean']->getDefault());
+
+ $diff = new TableDiff(
+ 'sqlsrv_df_constraints',
+ array(
+ new Column('df_current_timestamp', Type::getType('datetime'), array('default' => 'CURRENT_TIMESTAMP'))
+ ),
+ array(
+ 'df_integer' => new ColumnDiff(
+ 'df_integer',
+ new Column('df_integer', Type::getType('integer'), array('default' => 0)),
+ array('default'),
+ new Column('df_integer', Type::getType('integer'), array('default' => 666))
+ ),
+ 'df_string_2' => new ColumnDiff(
+ 'df_string_2',
+ new Column('df_string_2', Type::getType('string')),
+ array('default'),
+ new Column('df_string_2', Type::getType('string'), array('default' => 'Doctrine rocks!!!'))
+ ),
+ 'df_string_3' => new ColumnDiff(
+ 'df_string_3',
+ new Column('df_string_3', Type::getType('string'), array('length' => 50, 'default' => 'another default value')),
+ array('length'),
+ new Column('df_string_3', Type::getType('string'), array('length' => 50, 'default' => 'another default value'))
+ ),
+ 'df_boolean' => new ColumnDiff(
+ 'df_boolean',
+ new Column('df_boolean', Type::getType('boolean'), array('default' => false)),
+ array('default'),
+ new Column('df_boolean', Type::getType('boolean'), array('default' => true))
+ )
+ ),
+ array(
+ 'df_string_1' => new Column('df_string_1', Type::getType('string'))
+ ),
+ array(),
+ array(),
+ array(),
+ $table
+ );
+ $diff->newName = 'sqlsrv_default_constraints';
+ $diff->renamedColumns['df_string_4'] = new Column(
+ 'df_string_renamed',
+ Type::getType('string'),
+ array('default' => 'column to rename')
+ );
+
+ $this->_sm->alterTable($diff);
+ $columns = $this->_sm->listTableColumns('sqlsrv_default_constraints');
+
+ $this->assertNull($columns['no_default']->getDefault());
+ $this->assertEquals('CURRENT_TIMESTAMP', $columns['df_current_timestamp']->getDefault());
+ $this->assertEquals(0, $columns['df_integer']->getDefault());
+ $this->assertNull($columns['df_string_2']->getDefault());
+ $this->assertEquals('another default value', $columns['df_string_3']->getDefault());
+ $this->assertEquals(0, $columns['df_boolean']->getDefault());
+ $this->assertEquals('column to rename', $columns['df_string_renamed']->getDefault());
+
+ /**
+ * Test that column default constraints can still be referenced after table rename
+ */
+ $diff = new TableDiff(
+ 'sqlsrv_default_constraints',
+ array(),
+ array(
+ 'df_current_timestamp' => new ColumnDiff(
+ 'df_current_timestamp',
+ new Column('df_current_timestamp', Type::getType('datetime')),
+ array('default'),
+ new Column('df_current_timestamp', Type::getType('datetime'), array('default' => 'CURRENT_TIMESTAMP'))
+ ),
+ 'df_integer' => new ColumnDiff(
+ 'df_integer',
+ new Column('df_integer', Type::getType('integer'), array('default' => 666)),
+ array('default'),
+ new Column('df_integer', Type::getType('integer'), array('default' => 0))
+ )
+ ),
+ array(),
+ array(),
+ array(),
+ array(),
+ $table
+ );
+
+ $this->_sm->alterTable($diff);
+ $columns = $this->_sm->listTableColumns('sqlsrv_default_constraints');
+
+ $this->assertNull($columns['df_current_timestamp']->getDefault());
+ $this->assertEquals(666, $columns['df_integer']->getDefault());
+ }
}
View
18 tests/Doctrine/Tests/DBAL/Platforms/SQLServerPlatformTest.php
@@ -14,13 +14,13 @@ public function createPlatform()
public function getGenerateTableSql()
{
- return 'CREATE TABLE test (id INT IDENTITY NOT NULL, test NVARCHAR(255) NULL, PRIMARY KEY (id))';
+ return 'CREATE TABLE test (id INT IDENTITY NOT NULL, test NVARCHAR(255), PRIMARY KEY (id))';
}
public function getGenerateTableWithMultiColumnUniqueIndexSql()
{
return array(
- 'CREATE TABLE test (foo NVARCHAR(255) NULL, bar NVARCHAR(255) NULL)',
+ 'CREATE TABLE test (foo NVARCHAR(255), bar NVARCHAR(255))',
'CREATE UNIQUE INDEX UNIQ_D87F7E0C8C73652176FF8CAA ON test (foo, bar) WHERE foo IS NOT NULL AND bar IS NOT NULL'
);
}
@@ -28,11 +28,19 @@ public function getGenerateTableWithMultiColumnUniqueIndexSql()
public function getGenerateAlterTableSql()
{
return array(
- 'ALTER TABLE mytable ADD quota INT NULL',
+ 'ALTER TABLE mytable ADD quota INT',
'ALTER TABLE mytable DROP COLUMN foo',
- 'ALTER TABLE mytable ALTER COLUMN baz NVARCHAR(255) DEFAULT \'def\' NOT NULL',
- 'ALTER TABLE mytable ALTER COLUMN bloo BIT DEFAULT \'0\' NOT NULL',
+ 'ALTER TABLE mytable ALTER COLUMN baz NVARCHAR(255) NOT NULL',
+ "ALTER TABLE mytable ADD CONSTRAINT DF_6B2BD609_78240498 DEFAULT 'def' FOR baz",
+ 'ALTER TABLE mytable ALTER COLUMN bloo BIT NOT NULL',
"sp_RENAME 'mytable', 'userlist'",
+ "DECLARE @sql NVARCHAR(MAX) = N''; " .
+ "SELECT @sql += N'EXEC sp_rename N''' + dc.name + ''', N''' " .
+ "+ REPLACE(dc.name, '6B2BD609', 'E2B58069') + ''', ''OBJECT'';' " .
+ "FROM sys.default_constraints dc " .
+ "JOIN sys.tables tbl ON dc.parent_object_id = tbl.object_id " .
+ "WHERE tbl.name = 'userlist';" .
+ "EXEC sp_executesql @sql"
);
}
Something went wrong with that request. Please try again.