Issue #838992 by alexpott, Charlie ChX Negyesi, daffie, Damien Tournoud, sun,...

Issue #838992 by alexpott, Charlie ChX Negyesi, daffie, Damien Tournoud, sun, quietone, longwave: Change the uid field from integer to serial by leveraging NO_AUTO_VALUE_ON_ZERO on MySQL

core/lib/Drupal/Core/Command/DbDumpCommand.php

@@ -410,9 +410,15 @@ protected function getTemplate() {
 use Drupal\Core\Database\Database;
 
 $connection = Database::getConnection();
+// Ensure any tables with a serial column with a value of 0 are created as
+// expected.
+$sql_mode = $connection->query("SELECT @@sql_mode;")->fetchField();
+$connection->query("SET sql_mode = '$sql_mode,NO_AUTO_VALUE_ON_ZERO'");
 
 {{TABLES}}
 
+// Reset the SQL mode.
+$connection->query("SET sql_mode = '$sql_mode'");
 ENDOFSCRIPT;
     return $script;
   }

core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php

@@ -1057,7 +1057,14 @@ protected function mapToStorageRecord(ContentEntityInterface $entity, $table_nam
         // SQL database drivers.
         // @see https://www.drupal.org/node/2279395
         $value = SqlContentEntityStorageSchema::castValue($definition->getSchema()['columns'][$column_name], $value);
-        if (!(empty($value) && $this->isColumnSerial($table_name, $schema_name))) {
+        $empty_serial = empty($value) && $this->isColumnSerial($table_name, $schema_name);
+        // The user entity is a very special case where the ID field is a serial
+        // but we need to insert a row with an ID of 0 to represent the
+        // anonymous user.
+        // @todo https://drupal.org/i/3222123 implement a generic fix for all
+        //   entity types.
+        $user_zero = $this->entityTypeId === 'user' && $value === 0;
+        if (!$empty_serial || $user_zero) {
           $record->$schema_name = $value;
         }
       }

core/modules/user/src/UserStorage.php

@@ -18,21 +18,24 @@ class UserStorage extends SqlContentEntityStorage implements UserStorageInterfac
    * {@inheritdoc}
    */
   protected function doSaveFieldItems(ContentEntityInterface $entity, array $names = []) {
-    // The anonymous user account is saved with the fixed user ID of 0.
-    // Therefore we need to check for NULL explicitly.
-    if ($entity->id() === NULL) {
-      $entity->uid->value = $this->database->nextId($this->database->query('SELECT MAX([uid]) FROM {' . $this->getBaseTable() . '}')->fetchField());
-      $entity->enforceIsNew();
+    // The anonymous user account is saved with the fixed user ID of 0. MySQL
+    // does not support inserting an ID of 0 into serial field unless the SQL
+    // mode is set to NO_AUTO_VALUE_ON_ZERO.
+    // @todo https://drupal.org/i/3222123 implement a generic fix for all entity
+    //   types.
+    if ($entity->id() === 0) {
+      $database = \Drupal::database();
+      if ($database->databaseType() === 'mysql') {
+        $sql_mode = $database->query("SELECT @@sql_mode;")->fetchField();
+        $database->query("SET sql_mode = '$sql_mode,NO_AUTO_VALUE_ON_ZERO'");
+      }
     }
-    return parent::doSaveFieldItems($entity, $names);
-  }
+    parent::doSaveFieldItems($entity, $names);
 
-  /**
-   * {@inheritdoc}
-   */
-  protected function isColumnSerial($table_name, $schema_name) {
-    // User storage does not use a serial column for the user id.
-    return $table_name == $this->revisionTable && $schema_name == $this->revisionKey;
+    // Reset the SQL mode if we've changed it.
+    if (isset($sql_mode, $database)) {
+      $database->query("SET sql_mode = '$sql_mode'");
+    }
   }
 
   /**

core/modules/user/src/UserStorageSchema.php

@@ -26,16 +26,6 @@ protected function getEntitySchema(ContentEntityTypeInterface $entity_type, $res
     return $schema;
   }
 
-  /**
-   * {@inheritdoc}
-   */
-  protected function processIdentifierSchema(&$schema, $key) {
-    // The "users" table does not use serial identifiers.
-    if ($key != $this->entityType->getKey('id')) {
-      parent::processIdentifierSchema($schema, $key);
-    }
-  }
-
   /**
    * {@inheritdoc}
    */

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

+<?php
+
+namespace Drupal\Tests\user\Functional;
+
+use Drupal\FunctionalTests\Update\UpdatePathTestBase;
+
+// cSpell:ignore refobjid regclass attname attrelid attnum refobjsubid objid
+// cSpell:ignore classid
+
+/**
+ * Tests user_update_9301().
+ *
+ * @group user
+ */
+class UidUpdateToSerialTest extends UpdatePathTestBase {
+
+  /**
+   * {@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']);
+
+    $connection = \Drupal::database();
+    if ($connection->driver() == 'pgsql') {
+      $seq_name = $connection->makeSequenceName('users', 'uid');
+      $seq_owner = $connection->query("SELECT d.refobjid::regclass as table_name, a.attname as field_name
+        FROM pg_depend d
+        JOIN pg_attribute a ON a.attrelid = d.refobjid AND a.attnum = d.refobjsubid
+        WHERE d.objid = :seq_name::regclass
+        AND d.refobjsubid > 0
+        AND d.classid = 'pg_class'::regclass", [':seq_name' => 'public.' . $seq_name])->fetchObject();
+      $this->assertEquals($connection->tablePrefix('users') . 'users', $seq_owner->table_name);
+      $this->assertEquals('uid', $seq_owner->field_name);
+
+      $seq_last_value = $connection->query("SELECT last_value FROM $seq_name")->fetchField();
+      $maximum_uid = $connection->query('SELECT MAX([uid]) FROM {users}')->fetchField();
+      $this->assertEquals($maximum_uid + 1, $seq_last_value);
+    }
+  }
+
+}

core/modules/user/user.install

@@ -5,6 +5,8 @@
  * Install, update and uninstall functions for the user module.
  */
 
+use Drupal\Core\Entity\Sql\SqlContentEntityStorage;
+
 /**
  * Implements hook_schema().
  */
@@ -97,3 +99,38 @@ function user_install() {
 function user_update_last_removed() {
   return 8100;
 }
+
+/**
+ * Change the users table to use an serial uid field.
+ */
+function user_update_9301(&$sandbox) {
+  if (!\Drupal::entityTypeManager()->getStorage('user') instanceof SqlContentEntityStorage) {
+    return t('The user entity storage is not using an SQL storage, update skipped.');
+  }
+
+  $connection = \Drupal::database();
+  $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'");
+  }
+  $connection->schema()->changeField('users', 'uid', 'uid', ['type' => 'serial', 'not null' => TRUE], ['primary key' => ['uid']]);
+  if (isset($sql_mode)) {
+    $connection->query("SET sql_mode = '$sql_mode'");
+  }
+
+  // Update the last installed schema to reflect the change of field type.
+  $installed_storage_schema = \Drupal::keyValue('entity.storage_schema.sql');
+  $field_schema_data = $installed_storage_schema->get('user.field_schema_data.uid');
+  $field_schema_data['users']['fields']['uid']['type'] = 'serial';
+  $installed_storage_schema->set('user.field_schema_data.uid', $field_schema_data);
+
+  // The new PostgreSQL sequence for the uid field needs to start with the last
+  // used user ID + 1 and the sequence must be owned by uid field.
+  // @todo https://drupal.org/i/3028706 implement a generic fix.
+  if ($connection->driver() == 'pgsql') {
+    $maximum_uid = $connection->query('SELECT MAX([uid]) FROM {users}')->fetchField();
+    $seq = $connection->makeSequenceName('users', 'uid');
+    $connection->query("ALTER SEQUENCE " . $seq . " RESTART WITH " . ($maximum_uid + 1) . " OWNED BY {users}.uid");
+  }
+}

core/tests/Drupal/Tests/UpdatePathTestTrait.php

@@ -2,6 +2,7 @@
 
 namespace Drupal\Tests;
 
+use Drupal\Core\Database\Database;
 use Drupal\Core\Url;
 
 /**
@@ -107,6 +108,15 @@ protected function runUpdates($update_url = NULL) {
         $this->kernel->updateModules($module_handler_list, $module_handler_list);
       }
 
+      // Close any open database connections. This allows DB drivers that store
+      // static information to refresh it in the update runner.
+      // @todo https://drupal.org/i/3222121 consider doing this in
+      //   \Drupal\Core\DrupalKernel::initializeContainer() for container
+      //   rebuilds.
+      foreach (Database::getAllConnectionInfo() as $key => $info) {
+        Database::closeConnection(NULL, $key);
+      }
+
       // If we have successfully clicked 'Apply pending updates' then we need to
       // clear the caches in the update test runner as this has occurred as part
       // of the updates.