Raise the minimum MySQL version to 5.7.8 and MariaDB version to 10.2.7 in Drupal 9

Just to make sure tests can catch

For ref it introduced in https://git.drupalcode.org/project/drupal/commit/70dfc3bef9bdad7188c5754...

I think we should create a method that analyzes the version string and returns an array with the database type and the short version string.

If we gonna make this "detection method" then probably no driver split required.
Otoh the next logical step is populate "reserved keywords" per version string... Which is sole role of Requiments class for "capabilities detection"

If we gonna make this "detection method" then probably no driver split required.

What I think is going to happen is that we get site owners who think that MySQL and MariaDB are interchangeable. They going to use the setting that they have a MySQL database and then connect it to a MariaDB. And we do not catch this, it will result is some strange bugs.

I am not sure, but if I were to take a guess about what the wises decision would be, then I would not create a separate driver for MariaDB. AFAIK: Maybe we get a couple of cases where MariaDB needs to be treated a little bit different. With the version string that we get from the database we can determine if we have a MySQL or a MariaDB database. I do however want to have a minimum version MariaDB and a maximum version MariaDB in the testbot.

@Gabor: Sorry, I am not a native English speaker and I am also not the best communicator.

I think we can do it with one driver for MySQL and MariaDB. We shall need a detection method for determining whether it a MySQL or a MariaDB. Having two drivers has also disadvantages in site owner thinking that MariaDB is equivalent to MySQL database. I think we will have the least amount of problems if we do it with one driver. Both choices have their pro's and con's. In the long run we shall have to do two drivers (Drupal 10 or later).
We could build a check that when you select the MySQL or MariaDB driver that we test the database version string and then select for you the right driver.
If however the framework managers decide to for the two driver option that fine for me too.

Here's a patch that distinguishes between MySQL and MariaDB and updates their minimum required versions to those agreed upon. The method seems reliable, if indirect, and I don't see a better approach. Writing automated tests for this presents a unique challenge. @xjm suggests testing the patch as-is against now-unsupported database versions to confirm that it fails (proving that the new minimums are correctly enforced), and if we feel we need explicit coverage beyond that, writing tests that use database features absent from the latest unsupported versions. While the existing tests run, please voice your opinion about the need for new, explicit tests.

Oops. Wrong copy pasta.

I would like a separate helper method that takes as a parameter the database version and returns an array with the database type and the simplified database version number. That method can then be unit testable.

Here's a patch with a tested helper method. I'm not in favor of passing arrays around, and since I don't know of a need for a simplified version string anywhere, I didn't add code to provide it. (YAGNI, you know.) No word yet on getting MariaDB on the testbot.

Also, since the testbot blew up testing #15 on MySQL 5.5 (I don't know why it reported success in the UI), the essential functionality--rejecting unsupported versions appears to work in at least that case.

Coding standards fix.

Could we add testing for: https://stackoverflow.com/questions/56601304/what-does-the-first-part-of...

What sort of testing do you have in mind, @daffie? I assume you don't just mean this?

diff --git a/core/tests/Drupal/Tests/Core/Database/Driver/mysql/MysqlTasksTest.php b/core/tests/Drupal/Tests/Core/Database/Driver/mysql/MysqlTasksTest.php
index 5ebbc3cf1a..c68baacd5d 100644
--- a/core/tests/Drupal/Tests/Core/Database/Driver/mysql/MysqlTasksTest.php
+++ b/core/tests/Drupal/Tests/Core/Database/Driver/mysql/MysqlTasksTest.php
@@ -31,6 +31,7 @@ public function getDistributionFromVersion(string $version): string {
   public function providerGetDistributionFromVersion(): array {
     return [
       'MariaDB' => ['10.2.0-MariaDB', 'mariadb'],
+      'MariaDB w/ replication version hack' => ['5.5.5-10.2.20-MariaDB-1:10.2.20+maria~bionic', 'mariadb'],
       'MySQL' => ['5.7.28', 'mysql'],
       'Percona Server' => ['5.7.28-31', 'mysql'],
       'Default' => ['Literally anything else', 'mysql'],

@travisgrarden: The basic testing with a provider method is great. Thanks for that. The patch lloks great needs some more work:

1. +++ b/core/lib/Drupal/Core/Database/Driver/mysql/Install/Tasks.php
   @@ -50,11 +60,38 @@ public function name() {
   +  protected function getDistributionFromVersion(string $version): string {
   

   Could we get this method to return an array with the database type, which it does now and the short version number like 10.2.0.

2. +++ b/core/tests/Drupal/Tests/Core/Database/Driver/mysql/MysqlTasksTest.php
   @@ -0,0 +1,40 @@
   +      'MariaDB' => ['10.2.0-MariaDB', 'mariadb'],
   +      'MySQL' => ['5.7.28', 'mysql'],
   +      'Percona Server' => ['5.7.28-31', 'mysql'],
   +      'Default' => ['Literally anything else', 'mysql'],
   

   Could we change this to:

   +++ b/core/tests/Drupal/Tests/Core/Database/Driver/mysql/MysqlTasksTest.php
   @@ -0,0 +1,40 @@
   +      ['10.2.0-MariaDB', 'mariadb'],
   +      ['5.7.28', 'mysql'],
   +      ['5.7.28-31', 'mysql'],
   +      ['Literally anything else', 'mysql'],
   

   Adding the keys is not necessary and makes the provider method less readable.

3. Adding the line: "['5.5.5-10.2.20-MariaDB-1:10.2.20+maria~bionic', 'mariadb']," is what I would like.

4. +++ b/core/tests/Drupal/Tests/Core/Database/Driver/mysql/MysqlTasksTest.php
   @@ -0,0 +1,40 @@
   +  public function providerGetDistributionFromVersion(): array {
   

   The provider method need a docblock.

5. +++ b/core/tests/Drupal/Tests/Core/Database/Driver/mysql/MysqlTasksTest.php
   @@ -0,0 +1,40 @@
   +    $tasks = new class extends Tasks {
   

   In Drupal unit testing we use mocks. See: https://phpunit.de/manual/6.5/en/test-doubles.html.
   You can also look at other unit tests in Drupal core.

Thanks, @daffie. :)

1. +++ b/core/lib/Drupal/Core/Database/Driver/mysql/Install/Tasks.php
         @@ -50,11 +60,38 @@ public function name() {
         +  protected function getDistributionFromVersion(string $version): string {

   Could we get this method to return an array with the database type, which it does now and the short version number like 10.2.0.

   I don't perceive the need for this. There's no code anywhere that uses the short version number, and since this is an internal API (i.e., a protected method), there's no reason we can't just add another helper method in the future if the need arises.

2. +++ b/core/tests/Drupal/Tests/Core/Database/Driver/mysql/MysqlTasksTest.php
         @@ -0,0 +1,40 @@
         +      'MariaDB' => ['10.2.0-MariaDB', 'mariadb'],
         +      'MySQL' => ['5.7.28', 'mysql'],
         +      'Percona Server' => ['5.7.28-31', 'mysql'],
         +      'Default' => ['Literally anything else', 'mysql'],

   Could we change this to:

   +++ b/core/tests/Drupal/Tests/Core/Database/Driver/mysql/MysqlTasksTest.php
         @@ -0,0 +1,40 @@
         +      ['10.2.0-MariaDB', 'mariadb'],
         +      ['5.7.28', 'mysql'],
         +      ['5.7.28-31', 'mysql'],
         +      ['Literally anything else', 'mysql'],

   Adding the keys is not necessary and makes the provider method less readable.

   Sure, why not? :)

3. +++ b/core/tests/Drupal/Tests/Core/Database/Driver/mysql/MysqlTasksTest.php
         @@ -0,0 +1,40 @@
         +  public function providerGetDistributionFromVersion(): array {

   The provider method need a docblock.

   Done.

4. +++ b/core/tests/Drupal/Tests/Core/Database/Driver/mysql/MysqlTasksTest.php
         @@ -0,0 +1,40 @@
         +    $tasks = new class extends Tasks {

   In Drupal unit testing we use mocks. See: https://phpunit.de/manual/6.5/en/test-doubles.html.
   You can also look at other unit tests in Drupal core.

   I don't actually think this is a good case for a mock. The method under test is entirely isolated--it accesses nothing outside itself and has no side effects. How would a mock improve the coverage? As far as I can tell, it would only complicate the test for no benefit.

I don't actually think this is a good case for a mock.

I agree. Mocking is for replacing the test subject's dependencies, not for replacing the test subject itself.

+++ b/core/tests/Drupal/Tests/Core/Database/Driver/mysql/MysqlTasksTest.php
@@ -0,0 +1,46 @@
+    $tasks = new class extends Tasks {

Cool! I didn't know that PHP 7 added support for anonymous classes. I think this will be the first addition of that into core. Previously, for similar situations (changing the visibility of a method to public so we can test it), we added a named class to the test file. E.g., TestPluginBase within core/modules/views/tests/src/Kernel/Plugin/PluginBaseTest.php. But that was when we still supported PHP 5. Now that we require PHP 7, I see no downside to using an anonymous class here.

+++ b/core/tests/Drupal/Tests/Core/Database/Driver/mysql/MysqlTasksTest.php
@@ -0,0 +1,46 @@
+  public function testGetDistributionFromVersion($version, $expected): void {

Let's add a @covers here so we know what method is being covered. Also, I think we should add a test for the minimumVersion() method as well.

I think we should add a test for the minimumVersion() method as well.

Hm, maybe that's hard to unit test due to the dependency on Database::getConnection(). If so, I'd be fine with punting it to a followup that can explore how to dependency inject or otherwise mock that.

@TravisCarden: The patch looks good. Thanks.

At the moment we only need the database type/distribution (MySQL or MariaDB) for minimum version testing. With the growing number of differences between MySQL and MariaDB, it would be good to store the type/distribution value in a class variable of the Connection class from the MySQL driver. Therefor we also should move the method getDistributionFromVersion() to the class Connection from the MySQL driver. Adding a getter method to the Connection class is then also necessary. I am not sure if we can use the existing method databaseType() for this.

Re: #30, here's your requested @covers annotation, @effulgentsia.

Re: #32, @daffie, if there's such a growing number of differences between MySQL that we foresee forking more logic, doesn't it make more sense to go ahead and create a separate driver for MariaDB? (See #2985788: Add a separate MariaDB driver.) It's the textbook case for polymorphism. In fact, if we can't depend on feature parity and API equivalence between the two distributions for the foreseeable future, we have to separate them now to prevent either being constrained to a decaying object model or surprising people with a BC break in a minor version release. Right? Naturally, I'm open to being proved wrong. :)

@daffie, since the current patch works under present conditions and any significant increase in conditional logic will probably be handled in #2985788: Add a separate MariaDB driver, what if we go ahead and proceed with this as-is in the interests of keeping the code clean and moving on to spend our time on more pressing matters? We can comment on the other issue to make sure the distribution test is moved to somewhere more appropriate.

@TravisCarden: The problem that we face with MariaDB is that at the moment the database driver is the same as the one for MySQL. However we do not know how long that will last. Any new version from MySQL or MariaDB can change that. Any difference will propably be small. I would like Drupal core to be prepared for such an event.

Creating a seperate driver for MariaDB is a solution, but has its down sides for Drupal core. We would like to have the MariaDB driver to be dependent on the MySQL driver. Just like themes can be dependent on each other. However database drivers in Drupal core are not seperate entities, like themes are. Database drivers are fully merged in core (in the directory core/lib/Drupal/Core/Database/Driver). In other parts of core there also is specific code for MySQL. Just do a search for 'mysql' on the code of core. That all makes creating a new database driver that is depended on another not very staight forward.
What I also would like is to make database drivers into seperate entities, just like themes are. Move the by core supported drivers to core/databases and the contrib ones to databases.

And you are right @TravisCarden, that is a testbook case of polymorphism. When we have differences that will result in the MySQL and MariaDB drivers being different we shall need a seperate driver for MariaDB. I do not think that this will be the case for Drupal 9, but I do not know what the new releases of MySQL and MariaDB will bring. The pragmatic solution that I am suggestion is be prepared for such a difference and hope that it will not happen. If it happens it will propably be a small difference. Creating a seperate driver for MariaDB for Drupal9 after the release of Drupal 9 is not very end user/site owner friendly. Having possibly a bit of polymorphism as a fallback is something I can live with.

If you @TravisCarden or anybody else do not think that we should be prepared in such a way, then please say so.

I requeued #33 because that fail was unexpected given what the patch does.

I think for this issue, we should restrict the scope to implementing the agreed-upon version requirements for MySQL and MariaDB. @daffie, are you proposing that there's a preferred way to do that that creates less public API? Or something else? If we want to further decouple the MariaDB implementation from MySQL since they're diverging, we can do that in a followup, but I don't think it should block this issue since this is a hard requirement and breaking change for Drupal 9.

Back to NR since the patch passed as expected once the random fail was un-failed on the MySQL environment.

Not sure how #33 can work in practice. Yes, we get a platform dependent MIN version number from

+++ b/core/lib/Drupal/Core/Database/Driver/mysql/Install/Tasks.php
@@ -50,11 +60,38 @@ public function name() {
   public function minimumVersion() {
-    // This can not be increased above '5.5.5' without dropping support for all
-    // MariaDB versions. MariaDB prefixes its version string with '5.5.5-'. For
-    // more information, see
-    // https://github.com/MariaDB/server/blob/f6633bf058802ad7da8196d01fd19d75c53f7274/include/mysql_com.h#L42.
-    return '5.5.3';
+    $version = Database::getConnection()->version();
+    $distro = $this->getDistributionFromVersion($version);
+
+    if ($distro === 'mariadb') {
+      return self::MARIADB_MINIMUM_VERSION;
+    }
+
+    return self::MYSQL_MINIMUM_VERSION;
+  }

But this is checked at install time against the value returned from Connection::version in the install tasks:

  /**
   * Check the engine version.
   */
  protected function checkEngineVersion() {
    // Ensure that the database server has the right version.
    if ($this->minimumVersion() && version_compare(Database::getConnection()->version(), $this->minimumVersion(), '<')) {
      $this->fail(t("The database server version %version is less than the minimum required version %minimum_version.", ['%version' => Database::getConnection()->version(), '%minimum_version' => $this->minimumVersion()]));
    }
  }

in case of MariaDB, the value returned will always be 5.5.5-sth, for example 5.5.5-10.3.22-MariaDB-0+deb10u1.

So the checkEngineVersion in case of MariaDB will always fail, because

MIN VERSION == '10.2.0'
RUNNING VERSION == '5.5.5-10.3.22-MariaDB-0+deb10u1'

and 5.5.5 is semver lower than 10.2.0.

No?

So I think MySql driver's Connection::version should return a version dependent on the platform, and Tasks::minimumVersion should determine the minimum one by getting the value of \PDO::ATTR_SERVER_VERSION independently from Connection::version.

Oh, you're right, @mondrake; good catch! My test case was wrong: 5.5.5-10.2.20-MariaDB-1:10.2.20+maria~bionic should be considered MySQL, not MariaDB, due to its functional equivalence. I've fixed the test and made it pass. I believe my solution differed from your suggestion, but it's somewhat simpler and truer to the intention of the code and its comments, i.e., "MariaDB's version numbers diverged from MySQL's after 5.5... Earlier versions can be safely treated as MySQL."

I also refactored \Drupal\Core\Database\Install\Tasks::checkEngineVersion for testability and added coverage for the version comparison code it in order to eliminate any uncertainty there.

Oops. Coding standards.

Oh, and a namespace error.

Alternative approach, along the lines of #40.

OK, that is a different approach and I like it.
We still need testing for: Connection::version(), Tasks::name() and Tasks::minimumVersion().

+++ b/core/lib/Drupal/Core/Database/Driver/mysql/Connection.php
@@ -46,6 +46,15 @@ class Connection extends DatabaseConnection {
+   * MariaDB prefixes its version string with '5.5.5-'.

This isn't actually the case anymore, @mondrake. My local installation, for example, reports a version string of 10.4.11-MariaDB. I recommend using the regex from my patch, which causes MySQL-equivalent versions to fall back to MySQL. The test cases in my patch are valuable in this regard for exactly specifying the intended behavior.

#46 right so the criteria is that 'MariaDB' should be in the string, and '5.5.5-' just removed if it's at the beginning. We can still fix that in the regex, then add back the test. On it.

With a test for Connection::version.

+++ b/core/lib/Drupal/Core/Database/Driver/mysql/Install/Tasks.php
@@ -43,18 +54,30 @@ public function __construct() {
-    // MariaDB versions. MariaDB prefixes its version string with '5.5.5-'. For
-    // more information, see
+++ b/core/lib/Drupal/Core/Database/Driver/mysql/Connection.php
@@ -46,6 +46,16 @@ class Connection extends DatabaseConnection {
+   * MariaDB may prefix its version string with '5.5.5-', which should be
+   * ignored.

Anyone know when this changed? #46 reports that at least that installation of 10.4 wasn't prefixed with 5.5.5. But, are other installations of 10.4 prefixed that way? What about installations of 10.2 or 10.3?

The reason I ask is that this patch changes the version() method itself to report the non-prefixed version, which seems quite sensible. However, to what extent is that a BC break? In other words, is it currently possible that contrib might be doing something like:

if (version_compare($db->version(), 8) >= 0) {
  // Do something that's only supported on MySQL 8.
}

And relying on the fact that MariaDB 10.2 would not enter that if statement? But now, with this patch, it would?

Or, is it already the case that MariaDB 10.2 dropped that prefix (at least on some installations), which would mean that even with HEAD, contrib can't be relying on that?

If the latter, then this change doesn't represent a BC break, but if the former, then it does.

A more complete patch. I thought the returned string from version should include "all" the rest of the string, once '5.5.5-' is removed.

The reason I ask is that this patch changes the version() method itself to report the non-prefixed version, which seems quite sensible. However, to what extent is that a BC break? In other words, is it currently possible that contrib might be doing something like:

if (version_compare($db->version(), 8) >= 0) {
  // Do something that's only supported on MySQL 8.
}

If there are sites out there with a MariaDB backend and its version string is something like "5.5.5-10.2.20-MariaDB", then yes that site will break after the current patch is committed. The only way we can to prevent this is to create a seperate driver for MariaDB. The same sites only now with a version string like "10.1.20-MariaDB" are already failing. MySQL 8 has capabilities that are not supported by MariaDB 10.2.

On the website of MariaDB there are pages with the differences between MySQL and MariaDB. If you compare the list with the function differences: https://mariadb.com/kb/en/function-differences-between-mariadb-102-and-m... and https://mariadb.com/kb/en/function-differences-between-mariadb-105-and-m..., you will see that the list has grown significantly. The same for the system variable differences: https://mariadb.com/kb/en/system-variable-differences-between-mariadb-10... and https://mariadb.com/kb/en/system-variable-differences-between-mariadb-10... is also significantly grown. If we as the Drupal community cannot live with differences for methods like version(), then we should create a seperate MariaDB driver. If we can live with such differences then the patch looks good to me. All points from the previous comments are addressed and for me it is RTBC. We still need a testbot with MySQL 5.7 and a MariaDB 10.2 to change the status of this issue.

The same sites only now with a version string like "10.1.20-MariaDB" are already failing.

Do we know if those actually exist? Are there any builds of MariaDB 10.x that don't have a 5.y prefix in their version? What determines which MariaDB 10.x builds have the prefix and which don't? It probably makes sense to commit something like #51 regardless, but having this info will inform what we should write in the change record.

I have a few issues which it will be quicker/easier to create a patch for than to try to explain. I'll add one soon.

#53 it looks like the difference may come not from the server builds, rather from the linked client library - if that is mariadb aware then it will strip the 5.5.5 prefix before returning the value to PHP. See https://stackoverflow.com/questions/57496570/where-does-pdo-version-attr... and the github link from there.

Okay, here's my promised patch. It addresses the following concerns I alluded to:

1. The collaboration of Tasks and Connection smelled to me of feature envy on the one hand and leaky abstraction on the other. The repetition of the same regular expression multiple times wasn't DRY. I created a single, public isMariaDB() method on Connection to encapsulate the logic (and considerably simplify the tests).
2. The tests mocked the class under test, which should always be avoided. You may think I'm just looking for a chance to use an anonymous class (😛), but it's probably the best option in lieu of proper dependency injection, so that's what I did.
3. Least significantly, we were unit testing two classes in two namespaces in a single test, so I separated them.

I performed a strict and careful refactoring and ensured that @mondrake's original tests continued to pass until I refactored them, too.

+++ b/core/lib/Drupal/Core/Database/Driver/mysql/Install/Tasks.php
@@ -39,22 +51,48 @@ public function __construct() {
+  /**
+   * Returns the database connection.
+   *
+   * @return \Drupal\Core\Database\Driver\mysql\Connection
+   *   The database connection.
+   */
+  protected function getConnection() {
+    return Database::getConnection();
+  }
+
+  /**
+   * Translates a string to the current language or to a given language.
+   *
+   * @see \Drupal\Core\StringTranslation\TranslatableMarkup::__construct()
+   */
+  protected function t($string, array $args = [], array $options = []) {
+    return new TranslatableMarkup($string, $args, $options);
+  }
+
   /**

maybe these two methods could be moved to the abstract Drupal\Core\Database\Install\Tasks class, could be useful for other drivers, too.

There is a problem with using Azure Database for MySQL 8. The problem is that PDO::ATTR_SERVER_VERSION returns a value like '5.6.42.0'. Which is the version of the Azure Database and not of MySQL 8. If you run the query SELECT version() you get the version of MySQL and that is something like '8.0.15'. Because we are working on validating the MySQL/MariaDB version, maybe we can also fix it for Azure Database for MySQL 8. See: #3089902: "Azure Database for MySQL server" reports wrong database version.

I think #58 is out of scope for this issue and should be handled in a followup. Thanks @daffie!

Should I make another patch for @mondrake's suggestion in #57? Or can we RTBC this and move forward, creating a follow-up issue for Azure per @xjm's recommendation?

Here's a patch updated with @mondrake's feedback in #57.

The patch looks fine to me. Given we have unit tests with MariaDB version strings I think we could commit this now before the testbot is in place.

However this is still tagged for manual testing, is that still outstanding here or has someone done it?

Manually tested locally with MariaDB. Install was fine, status report looks like this.

Committed 39a5437 and pushed to 9.0.x. Thanks!

Yay! I'm happy to see this committed! Thanks for everyone who worked on it.

Can we raise the patch versions to the ones that support the JSON datatype? I can't think of a legitimate reason for anyone to be stuck on older patch versions.

Also, this patch updates INSTALL.txt.

That seems very reasonable to me. RTBC pending the bot.

+++ b/core/INSTALL.txt
@@ -51,10 +51,10 @@ Drupal requires:
-  - Percona Server 5.5.8 (or greater) (http://www.percona.com/). Percona
+  - Percona Server 5.7.8 (or greater) (http://www.percona.com/). Percona

Note that https://www.percona.com/doc/percona-server/5.7/release-notes/release-not... doesn't contain versions less than 5.7.10, but I'm assuming we should still specify 5.7.8 here to match the MySQL version number.

According to @Mixologic this is the commit that broke SQLite and Postgres, which is counterintuitive, but reverting and then re-running the patch on all DBs.

So, confirmed...

+++ b/core/lib/Drupal/Core/Database/Install/Tasks.php
@@ -300,4 +302,23 @@ public function validateDatabaseSettings($database) {
+  /**
+   * Translates a string to the current language or to a given language.
+   *
+   * @see \Drupal\Core\StringTranslation\TranslatableMarkup::__construct()
+   */
+  protected function t($string, array $args = [], array $options = []) {
+    return new TranslatableMarkup($string, $args, $options);
+  }
+
+  /**
+   * Returns the database connection.
+   *
+   * @return \Drupal\Core\Database\Connection
+   *   The database connection.
+   */
+  protected function getConnection() {
+    return Database::getConnection();
+  }

These might be the changes that broke it?

The installer calls thie ::name method while discovering the database drivers for all the drivers; the ::isMariaDb method is only defined for the MySql driver, so it will fail on other drivers.

Looks like #74 is still failing (maybe failing differently) for a number of other DBs. Possibly including MySQL 8?

Oops

I suspect the PHP 7.4 failure is due to the fact that SQLite was unbundled from PHP in that version, and it falls back to the distro sqlite3 library which is pretty old. It's a known problem, HEAD fails with PHP 7.4 and SQLite already.

EDIT - yes, see the IS of #3107155: Discuss lowering SQLite version requirement from 3.26 to 3.22 in Drupal 9 for detailed findings.

QuickStartTest must rely on sqlite even though we're running MySQL tests.

Also, the fix in #76 seems to work for all dbs on 7.3 now, however I am not sure it's the correct one - why should the installer call, given a driver, the Install/Tasks::name method of other drivers?

See #3107155-51: Discuss lowering SQLite version requirement from 3.26 to 3.22 in Drupal 9. I suggest to revert the SQLite bump. We need an issue to update the PHP 7.4 testbot contianers' sqlite3 library.

#81 right, but given each entry (=db driver) why aren't we calling only the name method of that driver, but also the other drivers' ones?

Trying a different approach re #80 @ #3113423-3: Ignore, testing issue.

The reason this is not passing on 7.4 was because of #3107155: Discuss lowering SQLite version requirement from 3.26 to 3.22 in Drupal 9, which has been reverted.

Let's see this. Interdiff coming shortly.

Interdiff. The failures in #63 were due, AFAICS, to the database settings of the main connection leaking in the test so that when Database::connection is called during the db driver discovery by the name method, the main connection object (not the one due to be installed) is activated and returned to the MySql install tasks. So here we are preventing activating the db connection and checking that the connection object is the MySql one before invoking isMariaDb.

Fixes to get unit tests pass.

#94 looks great! This is RTBC as far as I'm concerned. Incidentally, here's an interdiff between the previously committed patch and this one.

Actually, I guess I can actually RTBC this since I didn't work on the latest patch.

Looks great. Committed 28b8b4e and pushed to 9.0.x. Thanks!

Hopefully sticks this time!

I've just finished building the mariadb containers, have them deployed, and you'll see two tests currently running on #94.

The regex in the commit needs some work:

    There was 1 failure:

    1) Drupal\KernelTests\Core\Action\EmailActionTest::testEmailAction
    Failed to run installer database tasks: The database server version <em
    class="placeholder">10.2.7-MariaDB-10.2.7+maria~jessie</em>
    is less than the minimum required version <em
    class="placeholder">10.2.7</em>.

So, the mariadb official docker container is where it gets 10.2.7-MariaDB-10.2.7+maria~jessie So, not sure how many other variants we're going to see out there.

ah. of course the patch failed to apply. I'll run these against head.

Okay, maybe its not the regex. 10.3.22 seems to be working: https://www.drupal.org/pift-ci-job/1608199

But 10.2.7 definitely fails: https://dispatcher.drupalci.org/job/drupal8_core_regression_tests/15035/

I see a lot of already installed exceptions, so, not sure where thats coming from, but I still think it might be the regex.

Oops, this needs a CR. It also should go in the release notes.

Added the change record (and linked from the release note, although it doesn't really say anything extra).

Opened a follow-up: #3120124: Raise the minimum MariaDB version to 10.3(.7) in Drupal 9

I'm using Drupal 8.8.6. The version of SQLite3 is 3.7.11. Is it possible to upgrade to the latest version (3.32.1)?

I was able to upgrade the version on the console, but it remains 3.7.11 on the Drupal management screen.

This issue introduces methods in core/lib/Drupal/Core/Database/Driver/mysql/Connection.php to correct the database version when Mariadb is in use. The end effect is that the database version on the Status Report page and the Upgrade Status page is correct when viewed on Drupal 9, but is incorrect viewed on Drupal 8, on Pantheon. It seems to me that this issue is larger than Pantheon, and should also happen anywhere Mariadb is used.

Would it be possible to backport just the changes from core/lib/Drupal/Core/Database/Driver/mysql/Connection.php to Drupal 8.9.x? In my testing, doing this fixed the Mariadb version-reporting problem for both the Status Report page and the Upgrade Status page.

OK @Gábor Hojtsy, that is a good point. I'll work on a patch for the status report page on Drupal 8.9.x, and will put it in a new issue.

Created follow-on issue #3213482: Backport Mariadb version lookup from Drupal 9.x to Drupal 8.9.x