Issue #3255419 by alexpott, Bart Vanhoutte, daffie, catch, sam-elayyoub, roberto.muzzano, quietone, tstoeckler: Updating to Drupal 9.3 fails when sql_require_primary_key MySQL system variable is ON

core/modules/mysql/src/Driver/Database/mysql/Schema.php

@@ -634,6 +634,11 @@ class Schema extends DatabaseSchema {
       $sql .= ', ADD ' . implode(', ADD ', $keys_sql);
     }
     $this->connection->query($sql);
+
+    if ($spec['type'] === 'serial') {
+      $max = $this->connection->query('SELECT MAX(`' . $field_new . '`) FROM {' . $table . '}')->fetchField();
+      $this->connection->query("ALTER TABLE {" . $table . "} AUTO_INCREMENT = " . ($max + 1));
+    }
   }
 
   /**

core/modules/user/tests/src/Functional/Mysql8RequirePrimaryKeyUpdateTest.php

@@ -0,0 +1,65 @@
+<?php
+
+namespace Drupal\Tests\user\Functional;
+
+use Drupal\Core\Database\Database;
+use Drupal\FunctionalTests\Update\UpdatePathTestBase;
+
+/**
+ * Tests user_update_9301() on MySQL 8 when sql_require_primary_key is on.
+ *
+ * @group user
+ */
+class Mysql8RequirePrimaryKeyUpdateTest extends UpdatePathTestBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function runDbTasks() {
+    parent::runDbTasks();
+    $database = Database::getConnection();
+    $is_maria = method_exists($database, 'isMariaDb') && $database->isMariaDb();
+    if ($database->databaseType() !== 'mysql' || $is_maria || version_compare($database->version(), '8.0.13', '<')) {
+      $this->markTestSkipped('This test only runs on MySQL 8.0.13 and above');
+    }
+
+    $database->query("SET sql_require_primary_key = 1;")->execute();
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function prepareSettings() {
+    parent::prepareSettings();
+
+    // Set sql_require_primary_key for any future connections.
+    $settings['databases']['default']['default']['init_commands'] = (object) [
+      'value'    => ['sql_require_primary_key' => 'SET sql_require_primary_key = 1;'],
+      'required' => TRUE,
+    ];
+    $this->writeSettings($settings);
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setDatabaseDumpFiles() {
+    $this->databaseDumpFiles[] = __DIR__ . '/../../../../system/tests/fixtures/update/drupal-9.0.0.bare.standard.php.gz';
+  }
+
+  /**
+   * Tests user_update_9301().
+   */
+  public function testDatabaseLoaded() {
+    $key_value_store = \Drupal::keyValue('entity.storage_schema.sql');
+    $id_schema = $key_value_store->get('user.field_schema_data.uid', []);
+    $this->assertSame('int', $id_schema['users']['fields']['uid']['type']);
+
+    $this->runUpdates();
+
+    $key_value_store = \Drupal::keyValue('entity.storage_schema.sql');
+    $id_schema = $key_value_store->get('user.field_schema_data.uid', []);
+    $this->assertSame('serial', $id_schema['users']['fields']['uid']['type']);
+  }
+
+}

core/modules/user/user.install

@@ -113,12 +113,16 @@ function user_update_9301(&$sandbox) {
     return t('The Microsoft SQL Server does not support user_update_9301() because it causes data loss.');
   }
 
-  $connection->schema()->dropPrimaryKey('users');
   if ($connection->databaseType() === 'mysql') {
     $sql_mode = $connection->query("SELECT @@sql_mode;")->fetchField();
     $connection->query("SET sql_mode = '$sql_mode,NO_AUTO_VALUE_ON_ZERO'");
+    $new_keys = [];
   }
-  $connection->schema()->changeField('users', 'uid', 'uid', ['type' => 'serial', 'not null' => TRUE], ['primary key' => ['uid']]);
+  else {
+    $new_keys = ['primary key' => ['uid']];
+    $connection->schema()->dropPrimaryKey('users');
+  }
+  $connection->schema()->changeField('users', 'uid', 'uid', ['type' => 'serial', 'not null' => TRUE], $new_keys);
   if (isset($sql_mode)) {
     $connection->query("SET sql_mode = '$sql_mode'");
   }

core/tests/Drupal/KernelTests/Core/Database/SchemaTest.php

@@ -4,6 +4,7 @@ namespace Drupal\KernelTests\Core\Database;
 
 use Drupal\Component\Render\FormattableMarkup;
 use Drupal\Core\Database\Database;
+use Drupal\Core\Database\IntegrityConstraintViolationException;
 use Drupal\Core\Database\SchemaException;
 use Drupal\Core\Database\SchemaObjectDoesNotExistException;
 use Drupal\Core\Database\SchemaObjectExistsException;
@@ -924,6 +925,70 @@ class SchemaTest extends KernelTestBase {
     $this->schema->createTable($table_name, $table_spec);
   }
 
+  /**
+   * Tests converting an int to a serial when the int column has data.
+   */
+  public function testChangePrimaryKeyToSerial() {
+    // Test making an invalid field the primary key of the table upon creation.
+    $table_name = 'test_table';
+    $table_spec = [
+      'fields' => [
+        'test_field' => ['type' => 'int', 'not null' => TRUE],
+        'test_field_string'  => ['type' => 'varchar', 'length' => 20],
+      ],
+      'primary key' => ['test_field'],
+    ];
+    $this->schema->createTable($table_name, $table_spec);
+
+    if ($this->connection->databaseType() !== 'sqlite') {
+      try {
+        $this->connection
+          ->insert($table_name)
+          ->fields(['test_field_string' => 'test'])
+          ->execute();
+        $this->fail('Expected IntegrityConstraintViolationException not thrown');
+      }
+      catch (IntegrityConstraintViolationException $e) {
+      }
+    }
+
+    // @todo https://www.drupal.org/project/drupal/issues/3222127 Change the
+    //   first item to 0 to test changing a field with 0 to a serial.
+    // Create 8 rows in the table. Note that the 5 value is deliberately
+    // omitted.
+    foreach ([1, 2, 3, 4, 6, 7, 8, 9] as $value) {
+      $this->connection
+        ->insert($table_name)
+        ->fields(['test_field' => $value])
+        ->execute();
+    }
+    $this->schema->changeField($table_name, 'test_field', 'test_field', ['type' => 'serial', 'not null' => TRUE]);
+
+    $data = $this->connection
+      ->select($table_name)
+      ->fields($table_name, ['test_field'])
+      ->execute()
+      ->fetchCol();
+    $this->assertEquals([1, 2, 3, 4, 6, 7, 8, 9], array_values($data));
+
+    try {
+      $this->connection
+        ->insert($table_name)
+        ->fields(['test_field' => 1])
+        ->execute();
+      $this->fail('Expected IntegrityConstraintViolationException not thrown');
+    }
+    catch (IntegrityConstraintViolationException $e) {
+    }
+
+    // Ensure auto numbering now works.
+    $id = $this->connection
+      ->insert($table_name)
+      ->fields(['test_field_string' => 'test'])
+      ->execute();
+    $this->assertEquals(10, $id);
+  }
+
   /**
    * Tests adding an invalid field specification as a primary key.
    */