From 743d816631292a2081af4c5f7fc2fad2aff17c58 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 11 Apr 2014 17:08:01 +0200 Subject: [PATCH 01/13] [ticket/12012] Correctly drop default value constraints on MSSQL We need to drop the default constraints of a column, before being able to change their type or deleting them. PHPBB3-12012 --- phpBB/phpbb/db/tools.php | 144 ++++++++++++++++----------------------- 1 file changed, 59 insertions(+), 85 deletions(-) diff --git a/phpBB/phpbb/db/tools.php b/phpBB/phpbb/db/tools.php index 3d480b7e1c..87c205c6c0 100644 --- a/phpBB/phpbb/db/tools.php +++ b/phpBB/phpbb/db/tools.php @@ -1846,49 +1846,7 @@ class tools case 'mssql': case 'mssqlnative': - $sql = "SELECT CAST(SERVERPROPERTY('productversion') AS VARCHAR(25)) AS mssql_version"; - $result = $this->db->sql_query($sql); - $row = $this->db->sql_fetchrow($result); - $this->db->sql_freeresult($result); - - // Remove default constraints - if ($row['mssql_version'][0] == '8') // SQL Server 2000 - { - // http://msdn.microsoft.com/en-us/library/aa175912%28v=sql.80%29.aspx - // Deprecated in SQL Server 2005 - $statements[] = "DECLARE @drop_default_name VARCHAR(100), @cmd VARCHAR(1000) - SET @drop_default_name = - (SELECT so.name FROM sysobjects so - JOIN sysconstraints sc ON so.id = sc.constid - WHERE object_name(so.parent_obj) = '{$table_name}' - AND so.xtype = 'D' - AND sc.colid = (SELECT colid FROM syscolumns - WHERE id = object_id('{$table_name}') - AND name = '{$column_name}')) - IF @drop_default_name <> '' - BEGIN - SET @cmd = 'ALTER TABLE [{$table_name}] DROP CONSTRAINT [' + @drop_default_name + ']' - EXEC(@cmd) - END"; - } - else - { - $sql = "SELECT dobj.name AS def_name - FROM sys.columns col - LEFT OUTER JOIN sys.objects dobj ON (dobj.object_id = col.default_object_id AND dobj.type = 'D') - WHERE col.object_id = object_id('{$table_name}') - AND col.name = '{$column_name}' - AND dobj.name IS NOT NULL"; - $result = $this->db->sql_query($sql); - $row = $this->db->sql_fetchrow($result); - $this->db->sql_freeresult($result); - - if ($row) - { - $statements[] = 'ALTER TABLE [' . $table_name . '] DROP CONSTRAINT [' . $row['def_name'] . ']'; - } - } - + $statements = $this->mssql_drop_default_constraints($table_name, $column_name); $statements[] = 'ALTER TABLE [' . $table_name . '] DROP COLUMN [' . $column_name . ']'; break; @@ -2371,52 +2329,12 @@ class tools case 'mssql': case 'mssqlnative': + $statements = $this->mssql_drop_default_constraints($table_name, $column_name); $statements[] = 'ALTER TABLE [' . $table_name . '] ALTER COLUMN [' . $column_name . '] ' . $column_data['column_type_sql']; if (!empty($column_data['default'])) { - $sql = "SELECT CAST(SERVERPROPERTY('productversion') AS VARCHAR(25)) AS mssql_version"; - $result = $this->db->sql_query($sql); - $row = $this->db->sql_fetchrow($result); - $this->db->sql_freeresult($result); - - // Using TRANSACT-SQL for this statement because we do not want to have colliding data if statements are executed at a later stage - if ($row['mssql_version'][0] == '8') // SQL Server 2000 - { - $statements[] = "DECLARE @drop_default_name VARCHAR(100), @cmd VARCHAR(1000) - SET @drop_default_name = - (SELECT so.name FROM sysobjects so - JOIN sysconstraints sc ON so.id = sc.constid - WHERE object_name(so.parent_obj) = '{$table_name}' - AND so.xtype = 'D' - AND sc.colid = (SELECT colid FROM syscolumns - WHERE id = object_id('{$table_name}') - AND name = '{$column_name}')) - IF @drop_default_name <> '' - BEGIN - SET @cmd = 'ALTER TABLE [{$table_name}] DROP CONSTRAINT [' + @drop_default_name + ']' - EXEC(@cmd) - END - SET @cmd = 'ALTER TABLE [{$table_name}] ADD CONSTRAINT [DF_{$table_name}_{$column_name}_1] {$column_data['default']} FOR [{$column_name}]' - EXEC(@cmd)"; - } - else - { - $statements[] = "DECLARE @drop_default_name VARCHAR(100), @cmd VARCHAR(1000) - SET @drop_default_name = - (SELECT dobj.name FROM sys.columns col - LEFT OUTER JOIN sys.objects dobj ON (dobj.object_id = col.default_object_id AND dobj.type = 'D') - WHERE col.object_id = object_id('{$table_name}') - AND col.name = '{$column_name}' - AND dobj.name IS NOT NULL) - IF @drop_default_name <> '' - BEGIN - SET @cmd = 'ALTER TABLE [{$table_name}] DROP CONSTRAINT [' + @drop_default_name + ']' - EXEC(@cmd) - END - SET @cmd = 'ALTER TABLE [{$table_name}] ADD CONSTRAINT [DF_{$table_name}_{$column_name}_1] {$column_data['default']} FOR [{$column_name}]' - EXEC(@cmd)"; - } + $statements[] = 'ALTER TABLE [' . $table_name . '] ADD CONSTRAINT [DF_' . $table_name . '_' . $column_name . '_1] ' . $this->db->sql_escape($column_data['default']) . ' FOR [' . $column_name . ']'; } break; @@ -2551,4 +2469,60 @@ class tools return $this->_sql_run_sql($statements); } + + /** + * Drop the default constraints of a column + * + * We need to drop the default constraints of a column, + * before being able to change their type or deleting them. + * + * @param string $table_name + * @param string $column_name + * @return array Array with SQL statements + */ + protected function mssql_drop_default_constraints($table_name, $column_name) + { + $statements = array(); + $sql = "SELECT CAST(SERVERPROPERTY('productversion') AS VARCHAR(25)) AS mssql_version"; + $result = $this->db->sql_query($sql); + $mssql_server_properties = $this->db->sql_fetchrow($result); + $this->db->sql_freeresult($result); + + // Remove default constraints + if ($mssql_server_properties['mssql_version'][0] == '8') // SQL Server 2000 + { + // http://msdn.microsoft.com/en-us/library/aa175912%28v=sql.80%29.aspx + // Deprecated in SQL Server 2005 + $sql = "SELECT so.name AS def_name FROM sysobjects so + JOIN sysconstraints sc ON so.id = sc.constid + WHERE object_name(so.parent_obj) = '{$table_name}' + AND so.xtype = 'D' + AND sc.colid = (SELECT colid FROM syscolumns + WHERE id = object_id('{$table_name}') + AND name = '{$column_name}')"; + $result = $this->db->sql_query($sql); + while ($row = $this->db->sql_fetchrow($result)) + { + $statements[] = 'ALTER TABLE [' . $table_name . '] DROP CONSTRAINT [' . $row['def_name'] . ']'; + } + $this->db->sql_freeresult($result); + } + else + { + $sql = "SELECT dobj.name AS def_name + FROM sys.columns col + LEFT OUTER JOIN sys.objects dobj ON (dobj.object_id = col.default_object_id AND dobj.type = 'D') + WHERE col.object_id = object_id('{$table_name}') + AND col.name = '{$column_name}' + AND dobj.name IS NOT NULL"; + $result = $this->db->sql_query($sql); + while ($row = $this->db->sql_fetchrow($result)) + { + $statements[] = 'ALTER TABLE [' . $table_name . '] DROP CONSTRAINT [' . $row['def_name'] . ']'; + } + $this->db->sql_freeresult($result); + } + + return $statements; + } } From 29ba06968d383dd63a869345352117d7ed82497b Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sat, 12 Apr 2014 09:53:45 +0200 Subject: [PATCH 02/13] [ticket/12012] Fix query layout PHPBB3-12012 --- phpBB/phpbb/db/tools.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/phpBB/phpbb/db/tools.php b/phpBB/phpbb/db/tools.php index 87c205c6c0..a746099a14 100644 --- a/phpBB/phpbb/db/tools.php +++ b/phpBB/phpbb/db/tools.php @@ -2493,7 +2493,8 @@ class tools { // http://msdn.microsoft.com/en-us/library/aa175912%28v=sql.80%29.aspx // Deprecated in SQL Server 2005 - $sql = "SELECT so.name AS def_name FROM sysobjects so + $sql = "SELECT so.name AS def_name + FROM sysobjects so JOIN sysconstraints sc ON so.id = sc.constid WHERE object_name(so.parent_obj) = '{$table_name}' AND so.xtype = 'D' From 9036cdaaa2d45664bca1a0e0cdeaaf2bd13c662a Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sat, 12 Apr 2014 12:37:34 +0200 Subject: [PATCH 03/13] [ticket/12012] Drop and recreate indexes when changing a column on MSSQL PHPBB3-12012 --- phpBB/phpbb/db/tools.php | 111 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 111 insertions(+) diff --git a/phpBB/phpbb/db/tools.php b/phpBB/phpbb/db/tools.php index a746099a14..1299df51dd 100644 --- a/phpBB/phpbb/db/tools.php +++ b/phpBB/phpbb/db/tools.php @@ -1846,6 +1846,13 @@ class tools case 'mssql': case 'mssqlnative': + $indexes = $this->mssql_get_existing_indexes($table_name, $column_name); + + if (!empty($indexes)) + { + trigger_error("Column '$column_name' on table '$table_name' is still part of an index and can not be removed: " . implode(', ', array_keys($indexes)), E_USER_ERROR); + } + $statements = $this->mssql_drop_default_constraints($table_name, $column_name); $statements[] = 'ALTER TABLE [' . $table_name . '] DROP COLUMN [' . $column_name . ']'; break; @@ -2329,6 +2336,16 @@ class tools case 'mssql': case 'mssqlnative': + $indexes = $this->mssql_get_existing_indexes($table_name, $column_name); + + if (!empty($indexes)) + { + foreach ($indexes as $index_name => $index_data) + { + $this->sql_index_drop($table_name, $index_name); + } + } + $statements = $this->mssql_drop_default_constraints($table_name, $column_name); $statements[] = 'ALTER TABLE [' . $table_name . '] ALTER COLUMN [' . $column_name . '] ' . $column_data['column_type_sql']; @@ -2336,6 +2353,15 @@ class tools { $statements[] = 'ALTER TABLE [' . $table_name . '] ADD CONSTRAINT [DF_' . $table_name . '_' . $column_name . '_1] ' . $this->db->sql_escape($column_data['default']) . ' FOR [' . $column_name . ']'; } + + if (!empty($indexes)) + { + // Recreate indexes after we changed the column + foreach ($indexes as $index_name => $index_data) + { + $this->sql_create_index($table_name, $index_name, $index_data); + } + } break; case 'mysql_40': @@ -2526,4 +2552,89 @@ class tools return $statements; } + + /** + * Get a list with existing indexes for the column + * + * @param string $table_name + * @param string $column_name + * @return array Array with Index name => columns + */ + protected function mssql_get_existing_indexes($table_name, $column_name) + { + $existing_indexes = array(); + $sql = "SELECT CAST(SERVERPROPERTY('productversion') AS VARCHAR(25)) AS mssql_version"; + $result = $this->db->sql_query($sql); + $mssql_server_properties = $this->db->sql_fetchrow($result); + $this->db->sql_freeresult($result); + + // Remove default constraints + if ($mssql_server_properties['mssql_version'][0] == '8') // SQL Server 2000 + { + // http://msdn.microsoft.com/en-us/library/aa175912%28v=sql.80%29.aspx + // Deprecated in SQL Server 2005 + /** + * @todo Fix for SQL Server 2000 + $sql = "SELECT so.name AS def_name + FROM sysobjects so + JOIN sysconstraints sc ON so.id = sc.constid + WHERE object_name(so.parent_obj) = '{$table_name}' + AND so.xtype = 'D' + AND sc.colid = (SELECT colid FROM syscolumns + WHERE id = object_id('{$table_name}') + AND name = '{$column_name}')"; + $result = $this->db->sql_query($sql); + while ($row = $this->db->sql_fetchrow($result)) + { + $statements[] = 'ALTER TABLE [' . $table_name . '] DROP CONSTRAINT [' . $row['def_name'] . ']'; + } + $this->db->sql_freeresult($result); + */ + } + else + { + $sql = "SELECT DISTINCT ix.name AS phpbb_index_name + FROM sys.indexes ix + INNER JOIN sys.index_columns ixc + ON ixc.object_id = ix.object_id + AND ixc.index_id = ix.index_id + INNER JOIN sys.columns cols + ON cols.column_id = ixc.column_id + AND cols.object_id = ix.object_id + WHERE ix.object_id = object_id('{$table_name}') + AND cols.name = '{$column_name}'"; + $result = $this->db->sql_query($sql); + $existing_indexes = array(); + while ($row = $this->db->sql_fetchrow($result)) + { + $existing_indexes[$row['phpbb_index_name']] = array(); + } + $this->db->sql_freeresult($result); + + if (empty($existing_indexes)) + { + return array(); + } + + $sql = "SELECT DISTINCT ix.name AS phpbb_index_name, cols.name AS phpbb_column_name + FROM sys.indexes ix + INNER JOIN sys.index_columns ixc + ON ixc.object_id = ix.object_id + AND ixc.index_id = ix.index_id + INNER JOIN sys.columns cols + ON cols.column_id = ixc.column_id + AND cols.object_id = ix.object_id + WHERE ix.object_id = object_id('{$table_name}') + AND " . $this->db->sql_in_set('ix.name', array_keys($existing_indexes)); + $result = $this->db->sql_query($sql); + + while ($row = $this->db->sql_fetchrow($result)) + { + $existing_indexes[$row['phpbb_index_name']][] = $row['phpbb_column_name']; + } + $this->db->sql_freeresult($result); + } + + return $existing_indexes; + } } From e2784d01cee227a56f2ad7bcaee4b796dbf6dde6 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sat, 12 Apr 2014 12:42:32 +0200 Subject: [PATCH 04/13] [ticket/12012] Fix tools::mssql_get_existing_indexes() for SQL Server 2000 PHPBB3-12012 --- phpBB/phpbb/db/tools.php | 50 +++++++++++++++++++++++++++++----------- 1 file changed, 36 insertions(+), 14 deletions(-) diff --git a/phpBB/phpbb/db/tools.php b/phpBB/phpbb/db/tools.php index 1299df51dd..c2285bcf80 100644 --- a/phpBB/phpbb/db/tools.php +++ b/phpBB/phpbb/db/tools.php @@ -2568,28 +2568,50 @@ class tools $mssql_server_properties = $this->db->sql_fetchrow($result); $this->db->sql_freeresult($result); - // Remove default constraints if ($mssql_server_properties['mssql_version'][0] == '8') // SQL Server 2000 { // http://msdn.microsoft.com/en-us/library/aa175912%28v=sql.80%29.aspx // Deprecated in SQL Server 2005 - /** - * @todo Fix for SQL Server 2000 - $sql = "SELECT so.name AS def_name - FROM sysobjects so - JOIN sysconstraints sc ON so.id = sc.constid - WHERE object_name(so.parent_obj) = '{$table_name}' - AND so.xtype = 'D' - AND sc.colid = (SELECT colid FROM syscolumns - WHERE id = object_id('{$table_name}') - AND name = '{$column_name}')"; + $sql = "SELECT DISTINCT ix.name AS phpbb_index_name + FROM sysindexes ix + INNER JOIN sysindexkeys ixc + ON ixc.id = ix.id + AND ixc.indid = ix.indid + INNER JOIN syscolumns cols + ON cols.colid = ixc.colid + AND cols.id = ix.id + WHERE ix.id = object_id('{$table_name}') + AND cols.name = '{$column_name}'"; $result = $this->db->sql_query($sql); + $existing_indexes = array(); while ($row = $this->db->sql_fetchrow($result)) { - $statements[] = 'ALTER TABLE [' . $table_name . '] DROP CONSTRAINT [' . $row['def_name'] . ']'; + $existing_indexes[$row['phpbb_index_name']] = array(); + } + $this->db->sql_freeresult($result); + + if (empty($existing_indexes)) + { + return array(); + } + + $sql = "SELECT DISTINCT ix.name AS phpbb_index_name, cols.name AS phpbb_column_name + FROM sysindexes ix + INNER JOIN sysindexkeys ixc + ON ixc.id = ix.id + AND ixc.indid = ix.indid + INNER JOIN syscolumns cols + ON cols.colid = ixc.colid + AND cols.id = ix.id + WHERE ix.id = object_id('{$table_name}') + AND " . $this->db->sql_in_set('ix.name', array_keys($existing_indexes)); + $result = $this->db->sql_query($sql); + + while ($row = $this->db->sql_fetchrow($result)) + { + $existing_indexes[$row['phpbb_index_name']][] = $row['phpbb_column_name']; } $this->db->sql_freeresult($result); - */ } else { @@ -2600,7 +2622,7 @@ class tools AND ixc.index_id = ix.index_id INNER JOIN sys.columns cols ON cols.column_id = ixc.column_id - AND cols.object_id = ix.object_id + AND cols.object_id = ix.object_id WHERE ix.object_id = object_id('{$table_name}') AND cols.name = '{$column_name}'"; $result = $this->db->sql_query($sql); From 190b4282df5c5c81123b1ed371eeada385831e99 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sun, 13 Apr 2014 16:43:57 +0200 Subject: [PATCH 05/13] [ticket/12012] Return SQL statements for index drop/create Otherwise we recreate the index before changing the column PHPBB3-12012 --- phpBB/phpbb/db/tools.php | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/phpBB/phpbb/db/tools.php b/phpBB/phpbb/db/tools.php index c2285bcf80..113059900a 100644 --- a/phpBB/phpbb/db/tools.php +++ b/phpBB/phpbb/db/tools.php @@ -2336,21 +2336,32 @@ class tools case 'mssql': case 'mssqlnative': + // We need the data here + $old_return_statements = $this->return_statements; + $this->return_statements = true; + $indexes = $this->mssql_get_existing_indexes($table_name, $column_name); + // Drop any indexes if (!empty($indexes)) { foreach ($indexes as $index_name => $index_data) { - $this->sql_index_drop($table_name, $index_name); + $result = $this->sql_index_drop($table_name, $index_name); + $statements = array_merge($statements, $result); } } - $statements = $this->mssql_drop_default_constraints($table_name, $column_name); + // Drop default value constraint + $result = $this->mssql_drop_default_constraints($table_name, $column_name); + $statements = array_merge($statements, $result); + + // Change the column $statements[] = 'ALTER TABLE [' . $table_name . '] ALTER COLUMN [' . $column_name . '] ' . $column_data['column_type_sql']; if (!empty($column_data['default'])) { + // Add new default value constraint $statements[] = 'ALTER TABLE [' . $table_name . '] ADD CONSTRAINT [DF_' . $table_name . '_' . $column_name . '_1] ' . $this->db->sql_escape($column_data['default']) . ' FOR [' . $column_name . ']'; } @@ -2359,9 +2370,12 @@ class tools // Recreate indexes after we changed the column foreach ($indexes as $index_name => $index_data) { - $this->sql_create_index($table_name, $index_name, $index_data); + $result = $this->sql_create_index($table_name, $index_name, $index_data); + $statements = array_merge($statements, $result); } } + + $this->return_statements = $old_return_statements; break; case 'mysql_40': From a48f1abf6693eaee3343abd4f4a7e6412ecc10d9 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 14 Apr 2014 16:29:21 +0200 Subject: [PATCH 06/13] [ticket/12012] Add a unit test for changing the column type PHPBB3-12012 --- tests/dbal/db_tools_test.php | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/tests/dbal/db_tools_test.php b/tests/dbal/db_tools_test.php index e25335165a..0715b09ea3 100644 --- a/tests/dbal/db_tools_test.php +++ b/tests/dbal/db_tools_test.php @@ -11,7 +11,9 @@ require_once dirname(__FILE__) . '/../../phpBB/includes/functions.php'; class phpbb_dbal_db_tools_test extends phpbb_database_test_case { + /** @var \phpbb\db\driver\driver_interface */ protected $db; + /** @var \phpbb\db\tools */ protected $tools; protected $table_exists; protected $table_data; @@ -207,6 +209,32 @@ class phpbb_dbal_db_tools_test extends phpbb_database_test_case $this->assertFalse($this->tools->sql_column_exists('prefix_table_name', 'column_does_not_exist')); } + public function test_column_change_with_index() + { + // Create column + $this->assertFalse($this->tools->sql_column_exists('prefix_table_name', 'c_bug_12012')); + $this->assertTrue($this->tools->sql_column_add('prefix_table_name', 'c_bug_12012', array('DECIMAL', 0))); + $this->assertTrue($this->tools->sql_column_exists('prefix_table_name', 'c_bug_12012')); + + // Create index over the column + $this->assertFalse($this->tools->sql_index_exists('prefix_table_name', 'i_bug_12012')); + $this->assertTrue($this->tools->sql_create_index('prefix_table_name', 'i_bug_12012', array('c_bug_12012', 'c_bool'))); + $this->assertTrue($this->tools->sql_index_exists('prefix_table_name', 'i_bug_12012')); + + // Change type from int to string + $this->assertTrue($this->tools->sql_column_change('prefix_table_name', 'c_bug_12012', array('VCHAR:100', ''))); + + // Remove the index + $this->assertTrue($this->tools->sql_index_exists('prefix_table_name', 'i_bug_12012')); + $this->assertTrue($this->tools->sql_index_drop('prefix_table_name', 'i_bug_12012')); + $this->assertFalse($this->tools->sql_index_exists('prefix_table_name', 'i_bug_12012')); + + // Remove the column + $this->assertTrue($this->tools->sql_column_exists('prefix_table_name', 'c_bug_12012')); + $this->assertTrue($this->tools->sql_column_remove('prefix_table_name', 'c_bug_12012')); + $this->assertFalse($this->tools->sql_column_exists('prefix_table_name', 'c_bug_12012')); + } + public function test_column_remove() { $this->assertTrue($this->tools->sql_column_exists('prefix_table_name', 'c_int_size')); @@ -252,7 +280,7 @@ class phpbb_dbal_db_tools_test extends phpbb_database_test_case $this->assertFalse($this->tools->sql_table_exists('prefix_test_table')); } - public function test_peform_schema_changes_drop_tables() + public function test_perform_schema_changes_drop_tables() { $db_tools = $this->getMock('\phpbb\db\tools', array( 'sql_table_exists', @@ -278,7 +306,7 @@ class phpbb_dbal_db_tools_test extends phpbb_database_test_case )); } - public function test_peform_schema_changes_drop_columns() + public function test_perform_schema_changes_drop_columns() { $db_tools = $this->getMock('\phpbb\db\tools', array( 'sql_column_exists', From 540384890258a22860d655086521c968b30bcbea Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 14 Apr 2014 17:21:27 +0200 Subject: [PATCH 07/13] [ticket/12012] Add a unit test for removing a column with indexes PHPBB3-12012 --- tests/dbal/db_tools_test.php | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/dbal/db_tools_test.php b/tests/dbal/db_tools_test.php index 0715b09ea3..df8f22083b 100644 --- a/tests/dbal/db_tools_test.php +++ b/tests/dbal/db_tools_test.php @@ -244,6 +244,28 @@ class phpbb_dbal_db_tools_test extends phpbb_database_test_case $this->assertFalse($this->tools->sql_column_exists('prefix_table_name', 'c_int_size')); } + public function test_column_remove_with_index() + { + // Create column + $this->assertFalse($this->tools->sql_column_exists('prefix_table_name', 'c_bug_12012_2')); + $this->assertTrue($this->tools->sql_column_add('prefix_table_name', 'c_bug_12012_2', array('UINT', 4))); + $this->assertTrue($this->tools->sql_column_exists('prefix_table_name', 'c_bug_12012_2')); + + // Create index over the column + $this->assertFalse($this->tools->sql_index_exists('prefix_table_name', 'i_bug_12012_2')); + $this->assertTrue($this->tools->sql_create_index('prefix_table_name', 'i_bug_12012_2', array('c_bug_12012_2', 'c_bool'))); + $this->assertTrue($this->tools->sql_index_exists('prefix_table_name', 'i_bug_12012_2')); + + $this->assertFalse($this->tools->sql_index_exists('prefix_table_name', 'i_bug_12012_3')); + $this->assertTrue($this->tools->sql_create_index('prefix_table_name', 'i_bug_12012_3', array('c_bug_12012_2'))); + $this->assertTrue($this->tools->sql_index_exists('prefix_table_name', 'i_bug_12012_3')); + + // Remove the column + $this->assertTrue($this->tools->sql_column_exists('prefix_table_name', 'c_bug_12012_2')); + $this->assertTrue($this->tools->sql_column_remove('prefix_table_name', 'c_bug_12012_2')); + $this->assertFalse($this->tools->sql_column_exists('prefix_table_name', 'c_bug_12012_2')); + } + public function test_column_remove_primary() { $this->assertTrue($this->tools->sql_column_exists('prefix_table_name', 'c_id')); From 7dc163f2b7f483e4abb46015c0e41b47cddfd757 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 14 Apr 2014 17:21:53 +0200 Subject: [PATCH 08/13] [ticket/12012] Drop and recreate indexes when removing columns PHPBB3-12012 --- phpBB/phpbb/db/tools.php | 35 +++++++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/phpBB/phpbb/db/tools.php b/phpBB/phpbb/db/tools.php index 113059900a..bc505010ba 100644 --- a/phpBB/phpbb/db/tools.php +++ b/phpBB/phpbb/db/tools.php @@ -1846,15 +1846,46 @@ class tools case 'mssql': case 'mssqlnative': + // We need the data here + $old_return_statements = $this->return_statements; + $this->return_statements = true; + $indexes = $this->mssql_get_existing_indexes($table_name, $column_name); + // Drop any indexes + $recreate_indexes = array(); if (!empty($indexes)) { - trigger_error("Column '$column_name' on table '$table_name' is still part of an index and can not be removed: " . implode(', ', array_keys($indexes)), E_USER_ERROR); + foreach ($indexes as $index_name => $index_data) + { + $result = $this->sql_index_drop($table_name, $index_name); + $statements = array_merge($statements, $result); + if (sizeof($index_data) > 1) + { + // Remove this column from the index and recreate it + $recreate_indexes[$index_name] = array_diff($index_data, array($column_name)); + } + } } - $statements = $this->mssql_drop_default_constraints($table_name, $column_name); + // Drop default value constraint + $result = $this->mssql_drop_default_constraints($table_name, $column_name); + $statements = array_merge($statements, $result); + + // Remove the column $statements[] = 'ALTER TABLE [' . $table_name . '] DROP COLUMN [' . $column_name . ']'; + + if (!empty($recreate_indexes)) + { + // Recreate indexes after we removed the column + foreach ($recreate_indexes as $index_name => $index_data) + { + $result = $this->sql_create_index($table_name, $index_name, $index_data); + $statements = array_merge($statements, $result); + } + } + + $this->return_statements = $old_return_statements; break; case 'mysql_40': From a75ac5efd24694bf8edb85d0e4f8cb35853f94ca Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 16 Apr 2014 20:59:28 +0200 Subject: [PATCH 09/13] [ticket/12012] Handle begin and commit transactions in tests PHPBB3-12012 --- .../phpbb_database_test_connection_manager.php | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/tests/test_framework/phpbb_database_test_connection_manager.php b/tests/test_framework/phpbb_database_test_connection_manager.php index f6429b1ccb..431cee5588 100644 --- a/tests/test_framework/phpbb_database_test_connection_manager.php +++ b/tests/test_framework/phpbb_database_test_connection_manager.php @@ -14,6 +14,7 @@ class phpbb_database_test_connection_manager { private $config; private $dbms; + /** @var PDO */ private $pdo; /** @@ -363,9 +364,21 @@ class phpbb_database_test_connection_manager $table_name, $table_data ); + foreach ($queries as $query) { - $this->pdo->exec($query); + if ($query === 'begin') + { + $this->pdo->beginTransaction(); + } + else if ($query === 'commit') + { + $this->pdo->commit(); + } + else + { + $this->pdo->exec($query); + } } } } From 0a953ddb1505087f77eab1d2ee96033f99a08294 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sun, 20 Apr 2014 12:54:19 +0200 Subject: [PATCH 10/13] [ticket/12012] Remove duplicated code (only the $sql are different) PHPBB3-12012 --- phpBB/phpbb/db/tools.php | 103 ++++++++++++++++----------------------- 1 file changed, 43 insertions(+), 60 deletions(-) diff --git a/phpBB/phpbb/db/tools.php b/phpBB/phpbb/db/tools.php index bc505010ba..0e3ab59da0 100644 --- a/phpBB/phpbb/db/tools.php +++ b/phpBB/phpbb/db/tools.php @@ -2572,12 +2572,6 @@ class tools AND sc.colid = (SELECT colid FROM syscolumns WHERE id = object_id('{$table_name}') AND name = '{$column_name}')"; - $result = $this->db->sql_query($sql); - while ($row = $this->db->sql_fetchrow($result)) - { - $statements[] = 'ALTER TABLE [' . $table_name . '] DROP CONSTRAINT [' . $row['def_name'] . ']'; - } - $this->db->sql_freeresult($result); } else { @@ -2587,14 +2581,15 @@ class tools WHERE col.object_id = object_id('{$table_name}') AND col.name = '{$column_name}' AND dobj.name IS NOT NULL"; - $result = $this->db->sql_query($sql); - while ($row = $this->db->sql_fetchrow($result)) - { - $statements[] = 'ALTER TABLE [' . $table_name . '] DROP CONSTRAINT [' . $row['def_name'] . ']'; - } - $this->db->sql_freeresult($result); } + $result = $this->db->sql_query($sql); + while ($row = $this->db->sql_fetchrow($result)) + { + $statements[] = 'ALTER TABLE [' . $table_name . '] DROP CONSTRAINT [' . $row['def_name'] . ']'; + } + $this->db->sql_freeresult($result); + return $statements; } @@ -2627,36 +2622,6 @@ class tools AND cols.id = ix.id WHERE ix.id = object_id('{$table_name}') AND cols.name = '{$column_name}'"; - $result = $this->db->sql_query($sql); - $existing_indexes = array(); - while ($row = $this->db->sql_fetchrow($result)) - { - $existing_indexes[$row['phpbb_index_name']] = array(); - } - $this->db->sql_freeresult($result); - - if (empty($existing_indexes)) - { - return array(); - } - - $sql = "SELECT DISTINCT ix.name AS phpbb_index_name, cols.name AS phpbb_column_name - FROM sysindexes ix - INNER JOIN sysindexkeys ixc - ON ixc.id = ix.id - AND ixc.indid = ix.indid - INNER JOIN syscolumns cols - ON cols.colid = ixc.colid - AND cols.id = ix.id - WHERE ix.id = object_id('{$table_name}') - AND " . $this->db->sql_in_set('ix.name', array_keys($existing_indexes)); - $result = $this->db->sql_query($sql); - - while ($row = $this->db->sql_fetchrow($result)) - { - $existing_indexes[$row['phpbb_index_name']][] = $row['phpbb_column_name']; - } - $this->db->sql_freeresult($result); } else { @@ -2670,19 +2635,36 @@ class tools AND cols.object_id = ix.object_id WHERE ix.object_id = object_id('{$table_name}') AND cols.name = '{$column_name}'"; - $result = $this->db->sql_query($sql); - $existing_indexes = array(); - while ($row = $this->db->sql_fetchrow($result)) - { - $existing_indexes[$row['phpbb_index_name']] = array(); - } - $this->db->sql_freeresult($result); + } - if (empty($existing_indexes)) - { - return array(); - } + $result = $this->db->sql_query($sql); + $existing_indexes = array(); + while ($row = $this->db->sql_fetchrow($result)) + { + $existing_indexes[$row['phpbb_index_name']] = array(); + } + $this->db->sql_freeresult($result); + if (empty($existing_indexes)) + { + return array(); + } + + if ($mssql_server_properties['mssql_version'][0] == '8') // SQL Server 2000 + { + $sql = "SELECT DISTINCT ix.name AS phpbb_index_name, cols.name AS phpbb_column_name + FROM sysindexes ix + INNER JOIN sysindexkeys ixc + ON ixc.id = ix.id + AND ixc.indid = ix.indid + INNER JOIN syscolumns cols + ON cols.colid = ixc.colid + AND cols.id = ix.id + WHERE ix.id = object_id('{$table_name}') + AND " . $this->db->sql_in_set('ix.name', array_keys($existing_indexes)); + } + else + { $sql = "SELECT DISTINCT ix.name AS phpbb_index_name, cols.name AS phpbb_column_name FROM sys.indexes ix INNER JOIN sys.index_columns ixc @@ -2693,15 +2675,16 @@ class tools AND cols.object_id = ix.object_id WHERE ix.object_id = object_id('{$table_name}') AND " . $this->db->sql_in_set('ix.name', array_keys($existing_indexes)); - $result = $this->db->sql_query($sql); - - while ($row = $this->db->sql_fetchrow($result)) - { - $existing_indexes[$row['phpbb_index_name']][] = $row['phpbb_column_name']; - } - $this->db->sql_freeresult($result); } + $result = $this->db->sql_query($sql); + + while ($row = $this->db->sql_fetchrow($result)) + { + $existing_indexes[$row['phpbb_index_name']][] = $row['phpbb_column_name']; + } + $this->db->sql_freeresult($result); + return $existing_indexes; } } From 4775ad59b00631bfa3ff57ee8aaa025612301586 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sun, 20 Apr 2014 12:58:46 +0200 Subject: [PATCH 11/13] [ticket/12012] Fix docs in connection manager PHPBB3-12012 --- .../test_framework/phpbb_database_test_connection_manager.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_framework/phpbb_database_test_connection_manager.php b/tests/test_framework/phpbb_database_test_connection_manager.php index 431cee5588..887dad5b50 100644 --- a/tests/test_framework/phpbb_database_test_connection_manager.php +++ b/tests/test_framework/phpbb_database_test_connection_manager.php @@ -12,9 +12,11 @@ require_once dirname(__FILE__) . '/phpbb_database_connection_odbc_pdo_wrapper.ph class phpbb_database_test_connection_manager { + /** @var array */ private $config; + /** @var array */ private $dbms; - /** @var PDO */ + /** @var \PDO */ private $pdo; /** From bbc2e6c7b29603ab311828b34f3f55d0a44f0d7f Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 24 Apr 2014 14:53:33 +0200 Subject: [PATCH 12/13] [ticket/12012] Move MS SQL server comparison into a method PHPBB3-12012 --- phpBB/phpbb/db/tools.php | 46 ++++++++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/phpBB/phpbb/db/tools.php b/phpBB/phpbb/db/tools.php index 0e3ab59da0..9cd3e64d20 100644 --- a/phpBB/phpbb/db/tools.php +++ b/phpBB/phpbb/db/tools.php @@ -1869,7 +1869,7 @@ class tools } // Drop default value constraint - $result = $this->mssql_drop_default_constraints($table_name, $column_name); + $result = $this->mssql_get_drop_default_constraints_queries($table_name, $column_name); $statements = array_merge($statements, $result); // Remove the column @@ -2384,7 +2384,7 @@ class tools } // Drop default value constraint - $result = $this->mssql_drop_default_constraints($table_name, $column_name); + $result = $this->mssql_get_drop_default_constraints_queries($table_name, $column_name); $statements = array_merge($statements, $result); // Change the column @@ -2542,7 +2542,7 @@ class tools } /** - * Drop the default constraints of a column + * Get queries to drop the default constraints of a column * * We need to drop the default constraints of a column, * before being able to change their type or deleting them. @@ -2551,16 +2551,10 @@ class tools * @param string $column_name * @return array Array with SQL statements */ - protected function mssql_drop_default_constraints($table_name, $column_name) + protected function mssql_get_drop_default_constraints_queries($table_name, $column_name) { $statements = array(); - $sql = "SELECT CAST(SERVERPROPERTY('productversion') AS VARCHAR(25)) AS mssql_version"; - $result = $this->db->sql_query($sql); - $mssql_server_properties = $this->db->sql_fetchrow($result); - $this->db->sql_freeresult($result); - - // Remove default constraints - if ($mssql_server_properties['mssql_version'][0] == '8') // SQL Server 2000 + if ($this->mssql_is_sql_server_2000()) { // http://msdn.microsoft.com/en-us/library/aa175912%28v=sql.80%29.aspx // Deprecated in SQL Server 2005 @@ -2603,12 +2597,7 @@ class tools protected function mssql_get_existing_indexes($table_name, $column_name) { $existing_indexes = array(); - $sql = "SELECT CAST(SERVERPROPERTY('productversion') AS VARCHAR(25)) AS mssql_version"; - $result = $this->db->sql_query($sql); - $mssql_server_properties = $this->db->sql_fetchrow($result); - $this->db->sql_freeresult($result); - - if ($mssql_server_properties['mssql_version'][0] == '8') // SQL Server 2000 + if ($this->mssql_is_sql_server_2000()) { // http://msdn.microsoft.com/en-us/library/aa175912%28v=sql.80%29.aspx // Deprecated in SQL Server 2005 @@ -2650,7 +2639,7 @@ class tools return array(); } - if ($mssql_server_properties['mssql_version'][0] == '8') // SQL Server 2000 + if ($this->mssql_is_sql_server_2000()) { $sql = "SELECT DISTINCT ix.name AS phpbb_index_name, cols.name AS phpbb_column_name FROM sysindexes ix @@ -2687,4 +2676,25 @@ class tools return $existing_indexes; } + + protected $is_sql_server_2000; + + /** + * Is the used MS SQL Server a SQL Server 2000? + * + * @return bool + */ + protected function mssql_is_sql_server_2000() + { + if ($this->is_sql_server_2000 === null) + { + $sql = "SELECT CAST(SERVERPROPERTY('productversion') AS VARCHAR(25)) AS mssql_version"; + $result = $this->db->sql_query($sql); + $properties = $this->db->sql_fetchrow($result); + $this->db->sql_freeresult($result); + $this->is_sql_server_2000 = $properties['mssql_version'][0] == '8'; + } + + return $this->is_sql_server_2000; + } } From d5ea4906ca3a85a278518cc91189176174422bde Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 24 Apr 2014 22:57:35 +0200 Subject: [PATCH 13/13] [ticket/12012] Move property to the top PHPBB3-12012 --- phpBB/phpbb/db/tools.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/phpBB/phpbb/db/tools.php b/phpBB/phpbb/db/tools.php index 9cd3e64d20..2b0132075b 100644 --- a/phpBB/phpbb/db/tools.php +++ b/phpBB/phpbb/db/tools.php @@ -33,6 +33,12 @@ class tools */ var $dbms_type_map = array(); + /** + * Is the used MS SQL Server a SQL Server 2000? + * @var bool + */ + protected $is_sql_server_2000; + /** * Get the column types for every database we support * @@ -2677,8 +2683,6 @@ class tools return $existing_indexes; } - protected $is_sql_server_2000; - /** * Is the used MS SQL Server a SQL Server 2000? *