Problem/Motivation

The MS SQL Server fails on the user_update_9301(). The user IDs do not get properly set which results in the site being unavailable.

Proposed resolution

Given the update is causing data loss the first step here is to exit the function user_update_9301() when the database is SQL Server. This will probably prevent new users from being created on the site but it is far better outcome than losing the UID data.

The problem is caused by changing the UID field to serial - MSSQL does not support updating such fields so we can't preserve the value. This means user 0 becomes user 1 and there is no user 0 anymore. Also all numbers are sequential so if they are any gaps caused user deletion then we will lose this and the UIDs will be incorrect.

Remaining tasks

Follow-ups

Once this issue lands we will change #3263493: Upgrading to Drupal 9.3.x changes users table and locks users out of the site into providing an update for the UID field in the sqlsrv module. Other potential followups are to address sqlsrv's implementation of changeField() - at the very least it needs to throw an exception on conversion of a field to a serial as this does not work in the way it does for core db drivers. Potentially we need to add an explicit test for converting to a serial in core so that it is easy for alternate db drivers to implement support or know that they don't support this.

User interface changes

None

API changes

None

Data model changes

None

Release notes snippet

TBD

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

daffie created an issue. See original summary.

alexpott’s picture

Title: The SQL Server fails on user_update_9301() » user_update_9301() causes data loss and a broken site on SQL Server
Issue summary: View changes
alexpott’s picture

Issue summary: View changes
alexpott’s picture

daffie’s picture

Status: Active » Needs review
FileSize
657 bytes
alexpott’s picture

Status: Needs review » Needs work
+++ b/core/modules/user/user.install
@@ -109,6 +109,10 @@ function user_update_9301(&$sandbox) {
+    return t('The Microsoft SQL Server does not support updating the uid field to serial as it cannot preserve the field values.');

I think this text should be a bit more precise as we will have to update the sqlsrv module to provide it's own update.

So how about:

The Microsoft SQL Server does not support user_update_9301() because it causes data loss.
daffie’s picture

Status: Needs work » Needs review
FileSize
632 bytes

The patch with the new warning message.

alexpott’s picture

Status: Needs review » Reviewed & tested by the community

I've run the update on SQL Server and get the The Microsoft SQL Server does not support user_update_9301() because it causes data loss. message. I've run the update interactively and I'm not logged out and the site is working for anonymous users and I can log in again.

As expected, we're seeing the Mismatched entity and/or field definitions error on the status report. It says:

The following changes were detected in the entity type and field definitions.
User
The User ID field needs to be updated.

So that is correct.

Also as expected user creation fails with an exception. However this is all preferrable to the site breaking and data-loss.

catch’s picture

+++ b/core/modules/user/user.install
@@ -109,6 +109,10 @@ function user_update_9301(&$sandbox) {
   $connection = \Drupal::database();
+  if ($connection->databaseType() === 'sqlsrv') {
+    return t('The Microsoft SQL Server does not support user_update_9301() because it causes data loss.');
+  }
+

What about throwing an exception here - would be more obvious to the site owner that something is wrong.

More generally, what do we do about this update once sqlsrv is fixed? Does sqlsrv need to add its own update somewhere? Do we add a new core one?

alexpott’s picture

We're working on adding an update to sqlsrv for this in #3263493: Upgrading to Drupal 9.3.x changes users table and locks users out of the site. If we throw an exception then that'll stop all updates from running an not allow sqlsrv to provide an update.

  • catch committed d185ea1 on 9.4.x
    Issue #3265802 by daffie, alexpott: user_update_9301() causes data loss...

  • catch committed f23fbd6 on 9.3.x
    Issue #3265802 by daffie, alexpott: user_update_9301() causes data loss...
catch’s picture

Status: Reviewed & tested by the community » Fixed

OK if we're going to add an alternative update in sqlsrv itself rather than trying to rewrite the core update, then it makes sense to just skip relatively quietly here.

Committed/pushed to 9.4.x and cherry-picked to 9.3.x, thanks!

daffie’s picture

Assigned: daffie » Unassigned

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.