Merge d3338b9142 into 4367bc8d11

phpBB/phpbb/db/doctrine/connection_parameter_factory.php

@@ -149,6 +149,7 @@ class connection_parameter_factory
 		$enrichment_tags = [
 			'pdo_mysql' => [
 				'charset' => 'UTF8',
+				'platform' => new mysql_platform(),
 			],
 			'oci8' => [
 				'charset' => 'UTF8',

phpBB/phpbb/db/doctrine/mysql_platform.php

@@ -0,0 +1,62 @@
+<?php
+/**
+ *
+ * This file is part of the phpBB Forum Software package.
+ *
+ * @copyright (c) phpBB Limited <https://www.phpbb.com>
+ * @license GNU General Public License, version 2 (GPL-2.0)
+ *
+ * For full copyright and license information, please see
+ * the docs/CREDITS.txt file.
+ *
+ */
+
+namespace phpbb\db\doctrine;
+
+use Doctrine\DBAL\Platforms\AbstractMySQLPlatform;
+use Doctrine\DBAL\Schema\TableDiff;
+
+/**
+ * MySQL specific schema handling.
+ *
+ * While adding auto_increment column to MySQL, it must be indexed.
+ * If it's indexed as primary key, it should be declared as NOT NULL
+ * because MySQL primary key columns cannot be NULL.
+ */
+class mysql_platform extends AbstractMySQLPlatform
+{
+	/**
+	 * {@inheritDoc}
+	 */
+	public function getAlterTableSQL(TableDiff $diff)
+	{
+		$sql = parent::getAlterTableSQL($diff);
+		$table = $diff->getOldTable();
+		$columns = $diff->getAddedColumns();
+
+		foreach ($columns as $column)
+		{
+			$column_name = $column->getName();
+			if (!empty($column->getAutoincrement()) && $table)
+			{
+				foreach ($sql as $i => $query)
+				{
+					if (stripos($query, "add $column_name"))
+					{
+						if (!$table->getPrimaryKey())
+						{
+							$sql[$i] = str_replace(' DEFAULT NULL', '', $sql[$i]);
+							$sql[$i] .= ' PRIMARY KEY';
+						}
+						else
+						{
+							$sql[$i] .= ", ADD KEY ($column_name)";
+						}
+					}
+				}
+			}
+		}
+
+		return $sql;
+	}
+}

phpBB/phpbb/db/doctrine/postgresql_platform.php

@@ -18,6 +18,7 @@ use Doctrine\DBAL\Platforms\PostgreSQL94Platform;
 use Doctrine\DBAL\Schema\Index;
 use Doctrine\DBAL\Schema\Sequence;
 use Doctrine\DBAL\Schema\Table;
+use Doctrine\DBAL\Schema\TableDiff;
 use Doctrine\DBAL\Types\BigIntType;
 use Doctrine\DBAL\Types\IntegerType;
 use Doctrine\DBAL\Types\SmallIntType;
@@ -78,6 +79,36 @@ class postgresql_platform extends PostgreSQL94Platform
 		return AbstractPlatform::getDefaultValueDeclarationSQL($column);
 	}
 
+	/**
+	 * {@inheritDoc}
+	 */
+	public function getAlterTableSQL(TableDiff $diff)
+	{
+		$sql = parent::getAlterTableSQL($diff);
+		$table_name = $diff->getOldTable()->getName();
+		$columns = $diff->getAddedColumns();
+		$post_sql = $sequence_sql = [];
+
+		foreach ($columns as $column)
+		{
+			$column_name = $column->getName();
+			if (!empty($column->getAutoincrement()))
+			{
+				$sequence = new Sequence($this->getIdentitySequenceName($table_name, $column_name));
+				$sequence_sql[] = $this->getCreateSequenceSQL($sequence);
+				$post_sql[] = 'ALTER SEQUENCE '.$sequence->getName().' OWNED BY ' . $table_name . '.' . $column_name;
+			}
+		}
+		$sql = array_merge($sequence_sql, $sql, $post_sql);
+
+		foreach ($sql as $i => $query)
+		{
+			$sql[$i] = str_replace('{{placeholder_sequence}}', "nextval('{$table_name}_seq')", $query);
+		}
+
+		return $sql;
+	}
+
 	/**
 	 * {@inheritDoc}
 	 */

phpBB/phpbb/db/migration/data/v400/remove_jabber.php

@@ -34,7 +34,15 @@ class remove_jabber extends migration
 				$this->table_prefix . 'users' => [
 					'user_jabber',
 				],
-			]
+			],
+			'add_columns' => [
+				$this->table_prefix . 'user_notifications' => [
+					'id' => ['ULINT', null, 'auto_increment'],
+				],
+			],
+			'add_primary_keys' => [
+				$this->table_prefix . 'user_notifications' => ['id'],
+			],
 		];
 	}
 
@@ -45,7 +53,12 @@ class remove_jabber extends migration
 				$this->table_prefix . 'users' => [
 					'user_jabber' => ['VCHAR_UNI', ''],
 				],
-			]
+			],
+			'drop_columns' => [
+				$this->table_prefix . 'user_notifications' => [
+					'id',
+				],
+			],
 		];
 	}
 
@@ -105,11 +118,22 @@ class remove_jabber extends migration
 	{
 		$limit = 1000;
 
-		$sql = 'UPDATE ' . $this->tables['user_notifications'] . '
+		$sql = 'SELECT id FROM ' . $this->tables['user_notifications'] . "
-			SET ' . $this->db->sql_build_array('UPDATE', ['method' => 'notification.method.email']) . "
+			WHERE method = 'notification.method.jabber'
-			WHERE method = 'notification.method.jabber'";
+			ORDER BY id ASC";
-		$this->db->sql_query_limit($sql, $limit, $start ?: 0);
+		$result = $this->db->sql_query_limit($sql, $limit, $start ?: 0);
+		$rowset = $this->db->sql_fetchrowset($result);
+		$this->db->sql_freeresult($result);
+		$ids_array = array_column($rowset, 'id');
 
-		return $this->db->sql_affectedrows() < $limit ? true : $start + $limit;
+		if (count($ids_array))
+		{
+			$sql = 'UPDATE ' . $this->tables['user_notifications'] . '
+				SET ' . $this->db->sql_build_array('UPDATE', ['method' => 'notification.method.email']) . '
+				WHERE ' . $this->db->sql_in_set('id', $ids_array);
+			$this->db->sql_query($sql);
+		}
+
+		return count($ids_array) < $limit ? true : $start + $limit;
 	}
 }

phpBB/phpbb/db/migration/schema_generator.php

@@ -185,6 +185,7 @@ class schema_generator
 			'drop_columns'		=> 'COLUMNS',
 			'change_columns'	=> 'COLUMNS',
 			'add_index'			=> 'KEYS',
+			'add_primary_keys'	=> 'PRIMARY_KEY',
 			'add_unique_index'	=> 'KEYS',
 			'drop_keys'			=> 'KEYS',
 			'rename_index'		=> 'KEYS',

phpBB/phpbb/db/tools/doctrine.php

@@ -398,7 +398,7 @@ class doctrine implements tools_interface
 		return $this->alter_schema(
 			function (Schema $schema) use ($table_name, $column): void
 			{
-				$this->schema_create_primary_key($schema, $column, $table_name);
+				$this->schema_create_primary_key($schema, $table_name, $column);
 			}
 		);
 	}
@@ -990,16 +990,16 @@ class doctrine implements tools_interface
 	/**
 	 * Creates primary key for a table
 	 *
-	 * @param        $column
 	 * @param Schema $schema
 	 * @param string $table_name
+	 * @param array|string $column_name
 	 * @param bool   $safe_check
 	 *
 	 * @throws SchemaException
 	 */
-	protected function schema_create_primary_key(Schema $schema, $column, string $table_name, bool $safe_check = false): void
+	protected function schema_create_primary_key(Schema $schema, string $table_name, array|string $column_name, bool $safe_check = false): void
 	{
-		$columns = (is_array($column)) ? $column : [$column];
+		$columns = (is_array($column_name)) ? $column_name : [$column_name];
 		$table = $schema->getTable($table_name);
 		$table->dropPrimaryKey();
 		$table->setPrimaryKey($columns);

tests/dbal/migration/schema_add_autoincrement.php

@@ -0,0 +1,43 @@
+<?php
+/**
+*
+* This file is part of the phpBB Forum Software package.
+*
+* @copyright (c) phpBB Limited <https://www.phpbb.com>
+* @license GNU General Public License, version 2 (GPL-2.0)
+*
+* For full copyright and license information, please see
+* the docs/CREDITS.txt file.
+*
+*/
+
+class schema_add_autoincrement extends \phpbb\db\migration\migration
+{
+	function update_schema()
+	{
+		return [
+			'add_tables' => [
+				$this->table_prefix . 'noid' => [
+					'COLUMNS' => [
+						'text' => ['VCHAR:50', ''],
+					],
+				],
+			],
+
+			'add_columns' => [
+				$this->table_prefix . 'noid' => [
+					'id' => ['UINT:3', null, 'auto_increment'],
+				],
+			],
+		];
+	}
+
+	function revert_schema()
+	{
+		return [
+			'drop_tables'	=> [
+				$this->table_prefix . 'noid',
+			],
+		];
+	}
+}

tests/dbal/migrator_test.php

@@ -25,6 +25,7 @@ require_once __DIR__ . '/migration/fail.php';
 require_once __DIR__ . '/migration/installed.php';
 require_once __DIR__ . '/migration/schema.php';
 require_once __DIR__ . '/migration/schema_index.php';
+require_once __DIR__ . '/migration/schema_add_autoincrement.php';
 
 class phpbb_dbal_migrator_test extends phpbb_database_test_case
 {
@@ -502,4 +503,24 @@ class phpbb_dbal_migrator_test extends phpbb_database_test_case
 			}
 		}
 	}
+
+	public function test_add_autoincrement_column()
+	{
+		$this->migrator->set_migrations(['schema_add_autoincrement']);
+
+		while (!$this->migrator->finished())
+		{
+			$this->migrator->update();
+		}
+
+		$this->assertTrue($this->db_tools->sql_table_exists('phpbb_noid'));
+		$this->assertTrue($this->db_tools->sql_column_exists('phpbb_noid', 'id'));
+
+		while ($this->migrator->migration_state('schema_add_autoincrement'))
+		{
+			$this->migrator->revert('schema_add_autoincrement');
+		}
+
+		$this->assertFalse($this->db_tools->sql_table_exists('phpbb_noid'));
+	}
 }

tests/test_framework/phpbb_database_test_case.php

@@ -213,6 +213,10 @@ abstract class phpbb_database_test_case extends TestCase
 
 				if ($has_default_identity || $add_primary_keys)
 				{
+					// Enable implicit identity insert on mssql
+					$sql = "SET IDENTITY_INSERT $key OFF";
+					$db->sql_query($sql);
+
 					// Add default identity column to columns list
 					if ($has_default_identity)
 					{