Move core database drivers to modules of their own

There's some interesting conceptual things we need to work out.

• Is such a module enabled? The way the db code works atm that's not at all necessary.
• If such a module is installable and it's being used in the db driver then we need to mark it required because we don't want someone to think they can uninstall it

I think I'm happy for it to be a special case that database drivers are something from a module that does not need to be installed. But we certainly need to document that. So +1 to the code db driver modules being hidden as installing them makes no sense. Some db drivers have provided extra module like functionality and service replacements. They need installing.

+++ b/core/lib/Drupal/Core/Database/Install/Tasks.php
@@ -297,6 +297,13 @@ public function getFormOptions(array $database) {
+    $form['advanced_options']['autoload'] = [
+      '#type' => 'textfield',
+      '#title' => t('Autoload'),
+      '#default_value' => empty($database['autoload']) ? '' : $database['autoload'],
+      '#size' => 45,
+    ];

What is the use-case? This doesn't feel at all like something a user should ever configure. And also out-of-scope for this change.

#7 OTOH a non-core driver, in the current order, must implement at least a service to support date management functions via a backend overridable service. So another option would be for forcing at install time the installation of the ‘module’ that the driver resides in, and locking it as non-uninstallable.

also see #3128699-6: Testing issue for pgsql_fallback

+++ b/core/modules/system/tests/modules/driver_test/driver_test.info.yml
@@ -3,3 +3,7 @@ type: module
+dependencies:
+  - drupal:mysql
+  - drupal:pgsql

So this is where I have some issues. I think we need a proviso that drivers should be completely self contained and not rely on any module code. The way the container and therefore the autoloader is affected by the way we boot mandates this. The only reason the drivers in driver_test could extend the core drivers is because code in Drupal\Core is always autoloadable. Now it's in a place where it's not.

@daffie well we need to document it. It's enforced by the way db driver autoloading works. If you extend the core mysql connection class everything we jsut break because the system won't be able to autoload the core mysql connection class when it's in a module.

What I do not understand is how I can extend the by core supported database driver for PostgreSQL

You don't. You copy the classes and change what you need to. The classes shouldn't be viewed as API. They are an implementation of an API. If you copy the classes we can change core's implementation without ever having to worry about the impacts on contrib and custom. Which for something as low level as a db driver that's a very good thing.

@daffie

We enable the module that is the provider of the current database driver and all modules that it is dependent on.

As I've been trying to point out we shouldn't do this. Either the driver_test classes have a hard wired include to get around the autolaoding. Or we do not allow drivers to extend each other. And therefore driver_test classes need to have the driver specific classes too. #3124354: Remove unnecessary classes from the database drivers makes this a bit more palatable.

+++ b/core/modules/system/tests/modules/driver_test/src/Driver/Database/DrivertestMysql/Delete.php
@@ -2,6 +2,8 @@
+include_once dirname(__DIR__, 8) . '/mysql/src/Driver/Database/mysql/Delete.php';

neat... not seen the second arg to dirname - ah it was added in PHP 7 - nice and only quite recently possible for us to adopt.... we should tidy up all the dirname(dirname( in core :) and we definitely need #3124354: Remove unnecessary classes from the database drivers -

+++ b/core/composer.json
@@ -186,9 +189,6 @@
-            "lib/Drupal/Core/Database/Driver/mysql/Connection.php",
-            "lib/Drupal/Core/Database/Driver/pgsql/Connection.php",
-            "lib/Drupal/Core/Database/Driver/sqlite/Connection.php",

Hmmm it is a shame to lose this optimisation to class loading.

There's a big breaking change here - we moving classes out of core - and we're not deprecating them. That makes this change disruptive. The only way I can think of making this less disruptive is if the new modules provided aliases for the old classes. But even this is problematic because these aliases would only be available much later than the old classes would be. The other problem is that we can't leave the old classes there because then the drivers will be discovered.

If I apply this patch to an existing site it breaks because in settings.php I have

$databases['default']['default'] = array (
  'database' => 'drupal8alt',
  'username' => 'username',
  'password' => 'password',
  'prefix' => '',
  'host' => 'localhost',
  'port' => '',
  'namespace' => 'Drupal\\Core\\Database\\Driver\\mysql',
  'driver' => 'mysql',
);

So I see lots of

Warning: include(/Users/alex/dev/sites/drupal8alt.dev/vendor/composer/../../core/lib/Drupal/Core/Database/Driver/mysql/Connection.php): failed to open stream: No such file or directory in include() (line 444 of /Users/alex/dev/sites/drupal8alt.dev/vendor/composer/ClassLoader.php).

We need a BC layer to account for this.

Also we need to make the chosen database driver module required. The easiest way to do this will be in system_system_info_alter(). I think this might mean you can make the modules visible again since they won't be uninstallable if they are being used.

Just a couple of comments, skimming at the latest patch:

1. I think we should provide BC, cannot just move the classes. Maybe we can move the classes and still have an alias to them in place of current ones.
2. Why not to move all three drivers to a single module (even to the system module, maybe?) instead of one module per driver?

#3129534: Automatically enable the module that is providing the current database driver is in.

Not a native speaker, but I think this title is simpler :)

Thank you for answering my questions in #47, @daffie. That all makes sense.

The patch is great. The one thing that makes me wonder is updating existing sites - I see that an update function for instaling the driver module exists which is great, still the settings.php content for the database settings for new sites will be different from the one of existing sites. How do we go for that? do we keep a BC layer forever or are we going to inform the site admins that their setup is outdated at some point?

A few points, many nits, in detail:

1. +++ b/core/includes/install.inc
   @@ -174,7 +174,7 @@ function drupal_get_database_types() {
   -  $files = $file_system->scanDirectory(DRUPAL_ROOT . '/core/lib/Drupal/Core/Database/Driver', $mask, ['recurse' => FALSE]);
   +  $files = [];
      if (is_dir(DRUPAL_ROOT . '/drivers/lib/Drupal/Driver/Database')) {
        $files += $file_system->scanDirectory(DRUPAL_ROOT . '/drivers/lib/Drupal/Driver/Database/', $mask, ['recurse' => FALSE]);
      }
   

   it is purposeless to initalise $files to emtpy array, just make $files = instead of $files += two lines below

2. +++ b/core/lib/Drupal/Core/Database/Database.php
   @@ -461,6 +461,21 @@ public static function convertDbUrlToConnectionInfo($url, $root) {
   +    // Add the module key for the by core supported database drivers when the
   +    // module key is not set.
   

   see initial comment - shall we add a todo to remove this when we'll be able to assume that the module key is always set? or, can we somehow tamper with connection_info so that the key is always set when initialising?

3. +++ b/core/lib/Drupal/Core/Database/Database.php
   @@ -473,23 +488,18 @@ public static function convertDbUrlToConnectionInfo($url, $root) {
        if (!$module) {
          // Determine the connection class to use. Discover if the URL has a valid
   -      // driver scheme. Try with Drupal 8 style custom drivers first, since
   -      // those can override/extend the core ones.
   -      $connection_class = $custom_connection_class = "Drupal\\Driver\\Database\\{$driver}\\Connection";
   -      if (!class_exists($connection_class)) {
   -        // If the URL is not relative to a custom driver, try with core ones.
   -        $connection_class = "Drupal\\Core\\Database\\Driver\\{$driver}\\Connection";
   -      }
   +      // driver scheme for a Drupal 8 style custom driver.
   +      $connection_class = "Drupal\\Driver\\Database\\{$driver}\\Connection";
        }
    
        if (!class_exists($connection_class)) {
   

   same comment as above

4. +++ b/core/lib/Drupal/Core/Database/Database.php
   @@ -634,8 +644,8 @@ protected static function getDatabaseDriverNamespace(array $connection_info) {
   -    // Fallback for Drupal 7 settings.php.
   -    return 'Drupal\\Core\\Database\\Driver\\' . $connection_info['driver'];
   +    // Fallback for Drupal 7 settings.php when the namespace is not provided.
   +    return 'Drupal\\' . $connection_info['driver'] . '\\Driver\\Database\\' . $connection_info['driver'];
   

   Drupal 7 syntax still a problem?

5. +++ b/core/lib/Drupal/Core/Database/StatementInterface.php
   @@ -7,13 +7,13 @@
   + * class Drupal\oracle\Driver\Database\oracle\Statement extends PDOStatement implements Drupal\Core\Database\StatementInterface {}
   ...
   + * class Drupal\oracle\Driver\Database\oracle\Statement implements Iterator, Drupal\Core\Database\StatementInterface {}
   

   I'd use mymodule mydriver instead of oracle to make it clear in the example what's what

6. +++ b/core/lib/Drupal/Core/Site/Settings.php
   @@ -162,6 +162,43 @@ public static function initialize($app_root, $site_path, &$class_loader) {
   +        // Backwards compatibility layer for Drupal 8 style database connection
   +        // array's. Those do not have the autoload key set for the by Drupal
   +        // core supported database drivers.
   ...
   +        // Backwards compatibility layer for Drupal 8 style database connection
   +        // array's. Those have the wrong namespace key set or not set at all for
   +        // the by Drupal core supported database drivers.
   +        if (empty($info['namespace']) || (strpos($info['namespace'], 'Drupal\\Core\\Database\\Driver\\') === 0)) {
   +          switch (strtolower($info['driver'])) {
   

   see above

7. +++ b/core/modules/config/tests/src/Functional/ConfigImportAllTest.php
   @@ -87,10 +87,15 @@ public function testInstallUninstall() {
   +    sort($expected_modules);
   ...
   +    sort($validation_modules);
   

   no need to sort expected and actual values for assertEquals, you can use assertEqualsCanonicalising (I think)

8. +++ b/core/tests/Drupal/FunctionalTests/Installer/InstallerTest.php
   @@ -120,4 +123,31 @@ protected function visitInstaller() {
   +    $this->assertEquals(new Extension($this->root, 'module', "core/modules/$module/$module.info.yml", "$module.module"), $module_handler->getModule($module));
   

   I'd find more readable to associate new Extension(...) to a, say, $expected_extension local variable and use that in the assertEquals

+++ b/core/includes/install.inc
@@ -174,9 +174,9 @@ function drupal_get_database_types() {
+  $files = [];

this can be removed

#54.3 then I think we should have a @todo for removal in D10

#57 I missed that, sorry, OK then.

Any reason these would need to be "modules", especially as they have these weird properties of being hidden and not having to be installed. We already have multiple extension types, maybe a "dbdriver" type along with module, themes and profiles would be a cleaner way, not breaking module semantics about visibility and enabling/installing ?

I went through again and I think there could be a few more changes to be done; added in a diff file not break the patches flow and so they can be discussed. It's close to RTBC from me.

+++ b/core/lib/Drupal/Core/Database/Database.php
@@ -229,11 +229,6 @@ abstract class Database {
-    // Fallback for Drupal 7 settings.php when the namespace is not provided.
-    if (empty($info['namespace'])) {
-      $info['namespace'] = 'Drupal\\' . $info['driver'] . '\\Driver\\Database\\' . $info['driver'];
-    }
-

I think we can remove those given the changes in Settings

Actually, #62 does not work... :)

Will post again when I have a working diff

So, made some changes, mainly to ensure that Database::convertDbUrlToConnectionInfo() and Database::getConnectionInfoAsUrl() manage the 'module' addition both.

Had to take the patch from #2992274-18: Installer tests fail if contrib driver hides database credentials form fields to get this working.

At this stage, in a pure looking forward context, the 'namespace' key would be optional, only for BC, since any new db driver will be part of a module and therefore you can build the namespace with the 'module' and the 'driver' keys only. Maybe we can deprecate it?

working on some cleanup, @daffie hope you do not mind

the decision was made to use the 'namespace' and 'autoload' keys

oh I missed that. And your approach makes sense to me.

So I suppose my changes should be limited to the attached. No need for #2992274: Installer tests fail if contrib driver hides database credentials form fields since we are not adding the module key in createConnectionOptionsFromUrl.

Interdiff vs #59. Please disregard #62 and #65.

I think I can RTBC this, this patch is 99% @daffie's and I just made some minor changes.

Any reason these would need to be "modules", especially as they have these weird properties of being hidden and not having to be installed. We already have multiple extension types, maybe a "dbdriver" type along with module, themes and profiles would be a cleaner way, not breaking module semantics about visibility and enabling/installing ?

Profiles aren't a genuine third extension type, they're an extension of modules, issues like #820054: Add support for recommends[] and fix install profile dependency special casing, #3053521: Allow core.extension.yml to have empty profile on install site from existing configuration, #1676196: Install profiles are registered as modules are dealing with some of these inconsistencies.

If we made database drivers into a third extension type, then they'd still need to be able to register services on the container etc. (this is the reason they're being made modules at all) - and we'd still need them to be installable/uninstallable to be able to do this. So issues around running code when uninstalled, having them installable/uninstallable but hidden etc. would still be there - just a bit more siphoned into a different extension type. But that extension type would need to re-implement a lot of module functionality so it may not end up being any cleaner.

There's an old (won't fix) issue to allow themes to register services #2002606: Allow themes to provide services.yml. We ended up allowing themes to depend on modules instead #2937955: Themes can declare dependencies on modules.

Short version is I agree that them being modules adds some complexity/special casing to the module system, but I'm not sure trying to make them an independent extension type will remove any complexity overall - it's as likely to push it up into the extension system in general.

We might want a meta issue to look again at profiles vs. themes vs. modules (and now database drivers) though since those discussions are quite scattered and some are a decade old now.

1. +++ b/core/lib/Drupal/Core/Site/Settings.php
   @@ -162,6 +162,43 @@ public static function initialize($app_root, $site_path, &$class_loader) {
   +        // Backwards compatibility layer for Drupal 8 style database connection
   +        // array's. Those do not have the 'autoload' key set for core database
   +        // drivers.
   +        if (empty($info['autoload'])) {
   +          switch (strtolower($info['driver'])) {
   +            case 'mysql':
   +              $info['autoload'] = 'core/modules/mysql/src/Driver/Database/mysql/';
   +              break;
   +
   +            case 'pgsql':
   +              $info['autoload'] = 'core/modules/pgsql/src/Driver/Database/pgsql/';
   +              break;
   +
   +            case 'sqlite':
   +              $info['autoload'] = 'core/modules/sqlite/src/Driver/Database/sqlite/';
   +              break;
   +          }
   +        }
   +        // Backwards compatibility layer for Drupal 8 style database connection
   +        // array's. Those have the wrong 'namespace' key set, or not set at all
   +        // for core supported database drivers.
   +        if (empty($info['namespace']) || (strpos($info['namespace'], 'Drupal\\Core\\Database\\Driver\\') === 0)) {
   +          switch (strtolower($info['driver'])) {
   +            case 'mysql':
   +              $info['namespace'] = 'Drupal\\mysql\\Driver\\Database\\mysql';
   +              break;
   +
   +            case 'pgsql':
   +              $info['namespace'] = 'Drupal\\pgsql\\Driver\\Database\\pgsql';
   +              break;
   +
   +            case 'sqlite':
   +              $info['namespace'] = 'Drupal\\sqlite\\Driver\\Database\\sqlite';
   +              break;
   +          }
   +        }
   

   Do we need a follow-up to discuss how to deprecate this code and help people change their settings.php?

2. +++ b/core/modules/mysql/src/Driver/Database/mysql/Install/Tasks.php
   similarity index 98%
   rename from core/lib/Drupal/Core/Database/Driver/mysql/Schema.php
   
   rename from core/lib/Drupal/Core/Database/Driver/mysql/Schema.php
   rename to core/modules/mysql/src/Driver/Database/mysql/Schema.php
   
   rename to core/modules/mysql/src/Driver/Database/mysql/Schema.php
   index e05f10c86e..15752cbef0 100644
   
   index e05f10c86e..15752cbef0 100644
   --- a/core/lib/Drupal/Core/Database/Driver/mysql/Schema.php
   
   --- a/core/lib/Drupal/Core/Database/Driver/mysql/Schema.php
   +++ b/core/modules/mysql/src/Driver/Database/mysql/Schema.php
   
   +++ b/core/modules/mysql/src/Driver/Database/mysql/Schema.php
   +++ b/core/modules/mysql/src/Driver/Database/mysql/Schema.php
   @@ -1,6 +1,6 @@
   
   @@ -1,6 +1,6 @@
    <?php
    
   -namespace Drupal\Core\Database\Driver\mysql;
   +namespace Drupal\mysql\Driver\Database\mysql;
   

   We're moving these classes with no BC layer. That's a break.

   Is there any discussion of this on the issue? I think we need to leave a class here that extends the new class and is deprecated. In #24 I wrote...

   You copy the classes and change what you need to. The classes shouldn't be viewed as API. They are an implementation of an API. If you copy the classes we can change core's implementation without ever having to worry about the impacts on contrib and custom. Which for something as low level as a db driver that's a very good thing.

   which is true for the new module module classes but unfortunately I don't think is true for the existing classes which are part of the Drupal\Core namespace.

3. The patch also needs a reroll

@daffie I think if you have

[diff]
        renames = copies
        renameLimit = 7000

in your .gitconfig file you'll get a more reasonable overall patch size - when I recreate the patch it comes out at 339K - still big but I think better then maintaining multiple patches.

+++ b/core/lib/Drupal/Core/Database/Driver/mysql/Connection.php
@@ -0,0 +1,17 @@
+<?php
+
+namespace Drupal\Core\Database\Driver\mysql;
+
+use Drupal\mysql\Driver\Database\mysql\Connection as MysqlConnection;
+
+@trigger_error('\Drupal\Core\Database\Driver\mysql\Connection is deprecated in drupal:9.2.0 and is removed from drupal:10.0.0. The MySQL database driver has been moved to the mysql module. See https://www.drupal.org/node/3129492', E_USER_DEPRECATED);
+
+/**
+ * MySQL implementation of \Drupal\Core\Database\Connection.
+ *
+ * @deprecated in drupal:9.2.0 and is removed from drupal:10.0.0. The MySQL
+ *   database driver has been moved to the mysql module.
+ *
+ * @see https://www.drupal.org/node/3129492
+ */
+class Connection extends MysqlConnection {}

how about just using class_alias?

<?php

@trigger_error('\Drupal\Core\Database\Driver\mysql\Connection is deprecated in drupal:9.2.0 and is removed from drupal:10.0.0. The MySQL database driver has been moved to the mysql module. See https://www.drupal.org/node/3129492', E_USER_DEPRECATED);

class_alias("Drupal\mysql\Driver\Database\mysql\Connection", "Drupal\Core\Database\Driver\mysql\Connection");

#79 doesn't work unfortunately. I'd love to add a parameter to class_alias to trigger a user deprecation and then we could have file will class_aliases loaded as part of the static file map. See https://www.schmengler-se.de/en/2016/09/php-using-class_alias-to-maintai...

Ah sorry @daffie, thanks @alexpott. So it's not possible to alias at the same time as loading the potential class.. I missed that. IIUC #81 means that we could put the aliases in a deprecated class loading php file to be loaded by composer, but as things stand that would not allow for triggering deprecation errors. So the only option seems inheritance like #77 which implements that and adds tests.

I think this can go back to RTBC; #74.1 is for a followup I believe.

Re-rolled the patch in #85, thanks!

Alas, I am quite sure that commit of #3186934: Introduce an ExceptionHandler class in the database API, deprecate Connection::handleQueryException requires another reroll.

Added reroll of patch #87.

Fixed custom command errors in #90, thanks!

It'd probably be easier to move this issue to MR workflow.

I wonder whether #3214234: Add core/class_aliases.php could now make the class aliasing for BC simpler.

Rerolled and changed to MR workflow.

Thanks @daffie, I think the MR should be up to a working reroll of #93 now, with BC and deprecation tests included.

All fixed.

Ops.

Rerolled. This one is a pretty nasty one to reroll if there are changes to drivers' code in HEAD.

#107 I do, generally speaking. With this issue, the problem is that some classes are copied in a different directory structure AND changed at the source to keep the deprecation triggers. This does not work well with git on the repo, apparently: the changes to the 'old' sources are not tracked to the 'new' locations.

Filed #3231950: Split Database tests in 'core' ones and 'driver specific' ones as a D10 follow-up.

Rerolled

Fixing ComposerIntegrationTest::testComposerLockHash is a moving target, especially close to new releases when dependencies are updated frequently. I'd rather wait for a 'go' by core committers on the patch overall, and then do the final fix (along with any deprecation message update for versions).

Reroll after #3232699: SQLite schema introspection implementation struggles with NULL default values.

Rerolled

Rerolled

Rerolled

Rerolled

Changed target branch to 9.4.x.

Reroll

Updating deprecation messages.

Changed deprecation messages for deprecating in 9.4 and removing in 11.

Rerolled

Rerolled.

Rerolled

SQLite failure on PHP 8.1 is unrelated.

Should each core database’s DateSqlInterface implementations get moved into the respective modules as well?
For example:
core/modules/views/src/Plugin/views/query/SqliteDateSql.php

Running this branch with sqlsrv, I receive the following failure:

Drupal\FunctionalTests\Installer\InstallerTest::testInstalled
The file specified by the given app root, relative path and file name (/opt/drupal/core/modules/sqlsrv/sqlsrv.info.yml) do not exist. in /opt/drupal/core/lib/Drupal/Core/Extension/Extension.php:67

/opt/drupal/vendor/phpunit/phpunit/src/Framework/TestResult.php:737

Line 137 of InstallerTest.php is assuming that database drivers under test are in the core modules.

#129 yes! But I’d leave that to a follow-up, there will be lots of stuff to do after this gets committed. See for example #3231950: Split Database tests in 'core' ones and 'driver specific' ones.

Also, see #2657888: Add Date function support in DBTNG for an alternative approach.

Tests are now back green on all DBs in 9.4.

#130 looks like a problem that this patch needs to address - this should not be causing us to look in core for contrib database drivers.

My #130 issue has been resolved, so back to RTBC. Thanks for all the hard work!

Added a review with some actions to the MR.

The other thing I'm wondering about is migrations. How does this affect migrating from a different database provider eg. sqlite to mysql? This is mentioned in #61 but I can't glean the ramifications of this change on that.

Needs another reroll, unfortunately.

Also a requested comment in the code is missing, plus answer to #136, which I cannot answer.

Do not know if it's the same thing, but was wondering what would happen if a site had multiple database entries on different database types in settings.php. Would the update function need to enable ALL the modules affected?

This addresses #139, howeevr now I realise that probably calling \Database::getConnection() with key and target is possibly not a good idea, since there could be stale entries in $databases, and those would fail.

Addressed my own concern in #141, by avoiding actually opening connections.

I tested a slightly modified version of the MR with the DruDbal experimental db driver module, and it works correctly.

The change is in adding a couple of dump watches in UpdateEnableProviderDatabaseDriverTest, to inspect provider and list of installed modules before/after the post_update execution:

   public function testPostUpdateEnableProviderDatabaseDriverHook() {
     $connection = Database::getConnection();
     $provider = $connection->getProvider();
-
+dump([
+  '********** PRE',
+  'provider' => $provider,
+  'allConnectionsInfo' => Database::getAllConnectionInfo(),
+  'installed_modules' => implode(', ', array_keys(\Drupal::moduleHandler()->getModuleList())),
+]);
     $this->assertFalse(\Drupal::moduleHandler()->moduleExists($provider));

     // Running the updates enables the module that is providing the database
@@ -40,6 +45,10 @@ public function testPostUpdateEnableProviderDatabaseDriverHook() {
     $this->runUpdates();

     $this->assertTrue(\Drupal::moduleHandler()->moduleExists($provider));
+dump([
+  '********** POST',
+  'installed_modules' => implode(', ', array_keys(\Drupal::moduleHandler()->getModuleList())),
+]);
   }

 }

The test run result indicates that the contrib module gets installed too as part of the post_update:

Before:

array:4 [
  0 => "********** PRE"
  "provider" => "drudbal"
  "allConnectionsInfo" => array:2 [
    "simpletest_original_default" => array:1 [
      "default" => array:10 [
        "driver" => "dbal"
        "host" => "0.0.0.0"
        "database" => "drudbal"
        "prefix" => ""
        "namespace" => "Drupal\drudbal\Driver\Database\dbal"
        "port" => 3306
        "username" => "root"
        "password" => ""
        "dbal_driver" => "pdo_mysql"
        "autoload" => "modules/contrib/drudbal/src/Driver/Database/dbal/"
      ]
    ]
    "default" => array:1 [
      "default" => array:12 [
        "driver" => "dbal"
        "host" => "0.0.0.0"
        "database" => "drudbal"
        "prefix" => "test63429526"
        "namespace" => "Drupal\drudbal\Driver\Database\dbal"
        "port" => 3306
        "username" => "root"
        "password" => ""
        "dbal_driver" => "pdo_mysql"
        "autoload" => "modules/contrib/drudbal/src/Driver/Database/dbal/"
        "charset" => "utf8mb4"
        "init_commands" => array:1 [
          "sql_mode" => "SET sql_mode = 'ANSI,TRADITIONAL'"
        ]
      ]
    ]
  ]
  "installed_modules" => "automated_cron, block, block_content, breakpoint, ckeditor, color, comment, config, contact, contextual, datetime, dblog, editor, entity_reference, field, field_ui, file, filter, help, history, image, link, menu_ui, node, options, page_cache, path, path_alias, quickedit, rdf, search, shortcut, system, taxonomy, text, toolbar, tour, user, views_ui, menu_link_content, views, standard"
]

After:

array:2 [
  0 => "********** POST"
  "installed_modules" => "automated_cron, block, block_content, breakpoint, ckeditor, color, comment, config, contact, contextual, datetime, dblog, drudbal, editor, field, field_ui, file, filter, help, history, image, link, menu_ui, node, options, page_cache, path, path_alias, quickedit, rdf, search, shortcut, system, taxonomy, text, toolbar, tour, user, views_ui, menu_link_content, views, standard"
]

you can see the 'drudbal' module got installed.

Now, the test will probably not work with contrib drivers that are NOT encapsulated into a module yet, but to me this is more a benefit than not.

Viceversa, ExistingDrupal8StyleDatabaseConnectionInSettingsPhpTest can only reasonably work with mysql, pgsql or sqlite drivers.

Fixed #146.

Here's a patch for 10.0.x - there are conflicts in composer.lock and core/tests/Drupal/KernelTests/Core/Database/TransactionTest.php

Committed 934f42a and pushed to 10.0.x. Thanks!
Committed c18f8d6 and pushed to 9.4.x. Thanks!

A lot of great work here! But:

+++ b/core/lib/Drupal/Core/Site/Settings.php
@@ -162,6 +162,43 @@ public static function initialize($app_root, $site_path, &$class_loader) {
+        if (empty($info['autoload'])) {
+          switch (strtolower($info['driver'])) {
+            case 'mysql':
+              $info['autoload'] = 'core/modules/mysql/src/Driver/Database/mysql/';

This is too aggressive because it also autoloads to this directory for the Drupal\Driver namespace which is not sensible and creates errors for https://www.drupal.org/project/mysql56.

I opened #3284502: [regression] Drupal 9.4 breaks BC via incorrect autoloading of \Drupal\Driver\* overrides of core db drivers to fix that.

After installing the latest version of Drupal 9 for the security release today, I get this error on config import

The following reasons prevent the modules from being uninstalled: The module 'MySQL' is providing the database driver 'mysql'.

When I view my config differences, it shows that I'm removing mysql: 0 in core extensions. If I add mysql: 0 to my core extensions file, everything works normally.

Just to document this for others, this change has broken our sites on our hosting company Acquia due to them having some include logic that was including the old core DB drivers, I have a ticket with them to work on the issue. But other hosting companies may have similar problems.

After 9.4 were getting Error: Class "Drupal\mysql\Driver\Database\mysql\Connection" not found even with the new mysql module enabled.

function acquia_hosting_db_choose_active($db_info = NULL, $name = 'default', &$databases = NULL, &$conf = NULL) {
// Drupal 9 specific DB files.
require_once DRUPAL_ROOT . '/core/lib/Drupal/Core/Database/Database.php';
require_once DRUPAL_ROOT . '/core/lib/Drupal/Core/Database/Connection.php';

Note: This isn't a bug with 9.4 but just an unexpected side effect of this update having moved the location of mysql drivers

After a new build and new drush site:install for a new site ( not on update )

mkdir -p /var/www/html/dev
cd /var/www/html/dev/
composer create-project drupal/recommended-project d940x1 -vvv
cd /var/www/html/dev/d940x1/
composer require drush/drush
cd /var/www/html/dev/d940x1/web
../vendor/drush/drush/drush site:install standard --yes --account-name="webmaster" --account-pass="dD.123123ddd" --account-mail="webmaster@example.com" --db-url="mysql://root:rootpw@127.0.0.1/dev_d940x1" --locale="en"

Trying to do any other drush commands like
../vendor/drush/drush/drush user-create newuser --mail="person@example.com" --password="dD.123123ddd"

Facing issues with a number of contrib modules

[error]  TypeError: ############ Argument #1 ($database) must be of type Drupal\Core\Database\Driver\mysql\Connection, Drupal\mysql\Driver\Database\mysql\Connection given

But the Drupal\Core\Database\Driver\mysql\Connection is used not the Drupal\mysql\Driver\Database\mysql\Connection

And the message from the Mysql Driver Legacy Test

\\Drupal\\Core\\Database\\Driver\\mysql\\Connection is deprecated in drupal:9.4.0 and is removed from drupal:11.0.0. The MySQL database driver has been moved to the mysql module. See https://www.drupal.org/node/3129492'

The MySQL module

 drush en mysql
 [notice] Already enabled: MySQL

What is the next move for this change? Have that fixed in all used modules?
Change from Drupal\Core\Database\Driver\mysql\Connection to Drupal\mysql\Driver\Database\mysql\Connection

@Greg Boggs, @Rajab Natshah, @NicholasS, commenting on a closed issue won't help much because we don't receive notifications of updates for closed issues. Any time you encounter a regression from a change, you are better off opening a new critical or major bug report about it.

See these issues:

• #3285724: [regression] Drupal 9.4 breaks BC of \Drupal\Driver\* overriding core drivers during installation and parsing connection URLs
• #3284502: [regression] Drupal 9.4 breaks BC via incorrect autoloading of \Drupal\Driver\* overrides of core db drivers
• #3287251: Error updating from 9.3.16 to 9.4.0 with Autoupdates: Class 'Drupal\sqlite\Driver\Database\sqlite\Truncate' not found

If none of those address the problems you're encountering, open new issues referencing this one, and add the "9.4.0 update" issue tag to help us find it among regressions for this release.

Thanks for your reports!

I opened #3290924: [regression] With Drupal 9.4, can no longer call Database::getConnection() from within settings.php due to driver classes not yet in autoloader for #158. #159 might be a different issue though.

I opened #3290936: Argument #1 ($database) must be of type Drupal\Core\Database\Driver\mysql\Connection, Drupal\mysql\Driver\Database\mysql\Connection given for #159.

@Rajab Natshah: please comment there with any additional info you find about it. Also see my suggestion there to see if appending ?module=mysql fixes the site:install for you. E.g., --db-url="mysql://root:rootpw@127.0.0.1/dev_d940x1?module=mysql".

This is kind of the definition of a potentially disruptive change, and should have had a release note. We've added links to the related regressions to the release note, but we should also have one that explains the change itself, what will happen as a result, etc. A release note for it has also been requested in #3291224: Issues missing and broken link in 9.4.0 release notes.

Thanks!

Added a release note.

Linking the CR.

Per xjm's comment I've created a new issue: https://www.drupal.org/project/drupal/issues/3292510

In default.settings.php we have this comment:

 * The general format for the $databases array is as follows:
 * @code
 * $databases['default']['default'] = $info_array;
 * $databases['default']['replica'][] = $info_array;
 * $databases['default']['replica'][] = $info_array;
 * $databases['extra']['default'] = $info_array;
 * @endcode

Note the use of [] for replicas.
But it looks like the new code in \Drupal\Core\Site\Settings::initialize that adds in the namespaces, doesn't handle the array format

For some reason, sites in Acquia Site Factory (maybe Acquia Cloud Enterprise as well, not sure), this database update hook does not enable the MySQL module. It also doesn't get enabled when new sites are installed. It doesn't appear to be causing any problems, as I guess like I read in this issue, it's not technically required to have the module that provides a DB driver to be enabled. The status report page also doesn't show the error message indicating that the MySQL module needs to be installed.

This is because in Acquia, the 'autoload' key in the database connection string is not set. I don't know why, or how important that is. I opened a support ticket w/ Acquia just in case, because it seems like this might be problematic in the future.