Add support for native MySQL driver (mysqli) (#952)

.github/workflows/test-unit.yml

@@ -163,23 +163,41 @@ jobs:
           php -d opcache.enable_cli=1 vendor/bin/phpunit --exclude-group none $(if [ -n "$LOG_COVERAGE" ]; then echo --coverage-text; else echo --no-coverage; fi) -v
           if [ -n "$LOG_COVERAGE" ]; then mv coverage/phpunit.cov coverage/phpunit-sqlite.cov; fi
 
-      - name: "Run tests: MySQL"
+      - name: "Run tests: MySQL - PDO"
         env:
           DB_DSN: "mysql:host=mysql;dbname=atk4_test"
           DB_USER: atk4_test_user
           DB_PASSWORD: atk4_pass
         run: |
           php -d opcache.enable_cli=1 vendor/bin/phpunit --exclude-group none $(if [ -n "$LOG_COVERAGE" ]; then echo --coverage-text; else echo --no-coverage; fi) -v
-          if [ -n "$LOG_COVERAGE" ]; then mv coverage/phpunit.cov coverage/phpunit-mysql.cov; fi
+          if [ -n "$LOG_COVERAGE" ]; then mv coverage/phpunit.cov coverage/phpunit-mysql-pdo.cov; fi
 
-      - name: "Run tests: MariaDB"
+      - name: "Run tests: MySQL - mysqli"
+        env:
+          DB_DSN: "mysqli:host=mysql;dbname=atk4_test"
+          DB_USER: atk4_test_user
+          DB_PASSWORD: atk4_pass
+        run: |
+          php -d opcache.enable_cli=1 vendor/bin/phpunit --exclude-group none $(if [ -n "$LOG_COVERAGE" ]; then echo --coverage-text; else echo --no-coverage; fi) -v
+          if [ -n "$LOG_COVERAGE" ]; then mv coverage/phpunit.cov coverage/phpunit-mysql-mysqli.cov; fi
+
+      - name: "Run tests: MariaDB - PDO"
         env:
           DB_DSN: "mysql:host=mariadb;dbname=atk4_test"
           DB_USER: atk4_test_user
           DB_PASSWORD: atk4_pass
         run: |
           php -d opcache.enable_cli=1 vendor/bin/phpunit --exclude-group none $(if [ -n "$LOG_COVERAGE" ]; then echo --coverage-text; else echo --no-coverage; fi) -v
-          if [ -n "$LOG_COVERAGE" ]; then mv coverage/phpunit.cov coverage/phpunit-mariadb.cov; fi
+          if [ -n "$LOG_COVERAGE" ]; then mv coverage/phpunit.cov coverage/phpunit-mariadb-pdo.cov; fi
+
+      - name: "Run tests: MariaDB - mysqli"
+        env:
+          DB_DSN: "mysqli:host=mariadb;dbname=atk4_test"
+          DB_USER: atk4_test_user
+          DB_PASSWORD: atk4_pass
+        run: |
+          php -d opcache.enable_cli=1 vendor/bin/phpunit --exclude-group none $(if [ -n "$LOG_COVERAGE" ]; then echo --coverage-text; else echo --no-coverage; fi) -v
+          if [ -n "$LOG_COVERAGE" ]; then mv coverage/phpunit.cov coverage/phpunit-mariadb-mysqli.cov; fi
 
       - name: "Run tests: PostgreSQL"
         env:

phpstan.neon.dist

@@ -36,7 +36,7 @@ parameters:
             message: '~^(Call to an undefined method Doctrine\\DBAL\\Driver\\Connection::getWrappedConnection\(\)\.|Call to an undefined method Doctrine\\DBAL\\Connection::createSchemaManager\(\)\.|Call to an undefined static method Doctrine\\DBAL\\Exception::invalidPdoInstance\(\)\.|Call to method (getCreateTableSQL|getClobTypeDeclarationSQL|initializeCommentedDoctrineTypes)\(\) of deprecated class Doctrine\\DBAL\\Platforms\\\w+Platform:\n.+|Anonymous class extends deprecated class Doctrine\\DBAL\\Platforms\\(PostgreSQL94Platform|SQLServer2012Platform):\n.+|Call to deprecated method fetch(|All)\(\) of class Doctrine\\DBAL\\Result:\n.+|Call to deprecated method getSchemaManager\(\) of class Doctrine\\DBAL\\Connection:\n.+|Access to an undefined property Doctrine\\DBAL\\Driver\\PDO\\Connection::\$connection\.|Parameter #1 \$dsn of class Doctrine\\DBAL\\Driver\\PDO\\SQLSrv\\Connection constructor expects string, Doctrine\\DBAL\\Driver\\PDO\\Connection given\.|Method Atk4\\Data\\Persistence\\Sql\\Expression::execute\(\) should return Doctrine\\DBAL\\Result\|PDOStatement but returns bool\.|PHPDoc tag @return contains generic type Doctrine\\DBAL\\Schema\\AbstractSchemaManager<Doctrine\\DBAL\\Platforms\\AbstractPlatform> but class Doctrine\\DBAL\\Schema\\AbstractSchemaManager is not generic\.|Class Doctrine\\DBAL\\Platforms\\(MySqlPlatform|PostgreSqlPlatform) referenced with incorrect case: Doctrine\\DBAL\\Platforms\\(MySQLPlatform|PostgreSQLPlatform)\.)$~'
             path: '*'
             # count for DBAL 3.x matched in "src/Persistence/GenericPlatform.php" file
-            count: 39
+            count: 40
 
         # TODO these rules are generated, this ignores should be fixed in the code
         # for src/Schema/TestCase.php

src/Persistence.php

@@ -47,6 +47,7 @@ public static function connect($dsn, string $user = null, string $password = nul
         switch ($dsn['driver']) {
             case 'pdo_sqlite':
             case 'pdo_mysql':
+            case 'mysqli':
             case 'pdo_pgsql':
             case 'pdo_sqlsrv':
             case 'pdo_oci':

src/Persistence/GenericPlatform.php

@@ -47,6 +47,7 @@ private function createNotSupportedException(): \Exception
                 $connection->getSchemaManager();
                 $connection->getSchemaManager();
                 $connection->getSchemaManager();
+                $connection->getSchemaManager();
             }
         }
 

src/Persistence/Sql/Connection.php

@@ -8,6 +8,7 @@
 use Doctrine\Common\EventManager;
 use Doctrine\DBAL\Connection as DbalConnection;
 use Doctrine\DBAL\Driver\Connection as DbalDriverConnection;
+use Doctrine\DBAL\Driver\Mysqli\Connection as DbalMysqliConnection;
 use Doctrine\DBAL\DriverManager;
 use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\DBAL\Platforms\OraclePlatform;
@@ -35,6 +36,7 @@ abstract class Connection
     protected static $connectionClassRegistry = [
         'pdo_sqlite' => Sqlite\Connection::class,
         'pdo_mysql' => Mysql\Connection::class,
+        'mysqli' => Mysql\Connection::class,
         'pdo_pgsql' => Postgresql\Connection::class,
         'pdo_oci' => Oracle\Connection::class,
         'pdo_sqlsrv' => Mssql\Connection::class,
@@ -85,7 +87,7 @@ public static function normalizeDsn($dsn, $user = null, $password = null)
         if (isset($dsn['dsn'])) {
             if (str_contains($dsn['dsn'], '://')) {
                 $parsed = array_filter(parse_url($dsn['dsn']));
-                $dsn['dsn'] = $parsed['scheme'] . ':';
+                $dsn['dsn'] = str_replace('-', '_', $parsed['scheme']) . ':';
                 unset($parsed['scheme']);
                 foreach ($parsed as $k => $v) {
                     if ($k === 'pass') { // @phpstan-ignore-line phpstan bug
@@ -129,7 +131,7 @@ public static function normalizeDsn($dsn, $user = null, $password = null)
             $dsn['password'] = $password;
         }
 
-        if (!str_starts_with($dsn['driver'], 'pdo_')) {
+        if (!str_starts_with($dsn['driver'], 'pdo_') && !in_array($dsn['driver'], ['mysqli'], true)) {
             $dsn['driver'] = 'pdo_' . $dsn['driver'];
         }
 
@@ -197,14 +199,41 @@ final public static function isComposerDbal2x(): bool
         return !class_exists(DbalResult::class);
     }
 
-    private static function getDriverNameFromDbalDriverConnection(DbalDriverConnection $connection): string
+    /**
+     * @param DbalDriverConnection|DbalConnection $connection
+     */
+    private static function getDriverFromDbalDriverConnection($connection): object
     {
-        while (self::isComposerDbal2x() ? $connection instanceof \PDO : $connection = $connection->getWrappedConnection()) {
-            if ($connection instanceof \PDO) {
-                return 'pdo_' . $connection->getAttribute(\PDO::ATTR_DRIVER_NAME);
+        // TODO replace this method with Connection::getNativeConnection() once only DBAL 3.3+ is supported
+        // https://github.com/doctrine/dbal/pull/5037
+
+        if (self::isComposerDbal2x()) {
+            if ($connection instanceof \PDO || $connection instanceof \mysqli) {
+                return $connection;
             }
         }
 
+        $wrappedConnection = $connection instanceof DbalMysqliConnection
+            ? $connection->getWrappedResourceHandle()
+            : $connection->getWrappedConnection();
+
+        if ($wrappedConnection instanceof \PDO || $wrappedConnection instanceof \mysqli) {
+            return $wrappedConnection;
+        }
+
+        return self::getDriverFromDbalDriverConnection($wrappedConnection);
+    }
+
+    private static function getDriverNameFromDbalDriverConnection(DbalDriverConnection $connection): string
+    {
+        $driver = self::getDriverFromDbalDriverConnection($connection);
+
+        if ($driver instanceof \PDO) {
+            return 'pdo_' . $driver->getAttribute(\PDO::ATTR_DRIVER_NAME);
+        } elseif ($driver instanceof \mysqli) {
+            return 'mysqli';
+        }
+
         return null; // @phpstan-ignore-line
     }
 
@@ -216,7 +245,7 @@ protected static function createDbalEventManager(): EventManager
     protected static function connectFromDsn(array $dsn): DbalDriverConnection
     {
         $dsn = static::normalizeDsn($dsn);
-        if ($dsn['driver'] === 'pdo_mysql') {
+        if ($dsn['driver'] === 'pdo_mysql' || $dsn['driver'] === 'mysqli') {
             $dsn['charset'] = 'utf8mb4';
         } elseif ($dsn['driver'] === 'pdo_oci') {
             $dsn['charset'] = 'AL32UTF8';
@@ -413,7 +442,9 @@ public function rollBack(): void
      */
     public function lastInsertId(string $sequence = null): string
     {
-        return $this->connection()->lastInsertId($sequence);
+        $res = $this->connection()->lastInsertId($sequence);
+
+        return is_int($res) ? (string) $res : $res;
     }
 
     public function getDatabasePlatform(): AbstractPlatform

src/Persistence/Sql/Expression.php

@@ -493,6 +493,41 @@ public function __debugInfo(): array
         return $arr;
     }
 
+    protected function hasNativeNamedParamSupport(): bool
+    {
+        return true;
+    }
+
+    /**
+     * @param array{string, array<string, mixed>} $render
+     *
+     * @return array{string, array<string, mixed>}
+     *
+     * @internal
+     */
+    protected function updateRenderBeforeExecute(array $render): array
+    {
+        [$sql, $params] = $render;
+
+        if (!$this->hasNativeNamedParamSupport()) {
+            $numParams = [];
+            $i = 0;
+            $j = 0;
+            $sql = preg_replace_callback(
+                '~(?:\'(?:\'\'|\\\\\'|[^\'])*\')?+\K(?:\?|:\w+)~s',
+                function ($matches) use ($params, &$numParams, &$i, &$j) {
+                    $numParams[++$i] = $params[$matches[0] === '?' ? ++$j : $matches[0]];
+
+                    return '?';
+                },
+                $sql
+            );
+            $params = $numParams;
+        }
+
+        return [$sql, $params];
+    }
+
     /**
      * @param DbalConnection|Connection $connection
      *
@@ -506,7 +541,7 @@ public function execute(object $connection = null): object
 
         // If it's a DBAL connection, we're cool
         if ($connection instanceof DbalConnection) {
-            [$query, $params] = $this->render();
+            [$query, $params] = $this->updateRenderBeforeExecute($this->render());
 
             $platform = $this->connection->getDatabasePlatform();
             try {
@@ -575,11 +610,13 @@ public function execute(object $connection = null): object
                 }
                 $errorInfo = $firstException instanceof \PDOException ? $firstException->errorInfo : null;
 
-                $new = (new ExecuteException('Dsql execute error', $errorInfo[1] ?? 0, $e))
-                    ->addMoreInfo('error', $errorInfo[2] ?? 'n/a (' . $errorInfo[0] . ')')
-                    ->addMoreInfo('query', $this->getDebugQuery());
+                $eNew = (new ExecuteException('Dsql execute error', $errorInfo[1] ?? $e->getCode(), $e));
+                if ($errorInfo !== null && $errorInfo !== []) {
+                    $eNew->addMoreInfo('error', $errorInfo[2] ?? 'n/a (' . $errorInfo[0] . ')');
+                }
+                $eNew->addMoreInfo('query', $this->getDebugQuery());
 
-                throw $new;
+                throw $eNew;
             }
         }
 

src/Persistence/Sql/Mssql/ExpressionTrait.php

@@ -30,37 +30,11 @@ public function render(): array
             return $matches[1] . (!in_array($matches[1], ['N', '\'', '\\'], true) ? 'N' : '') . $matches[2];
         }, $sql);
 
-        // MSSQL does not support named parameters, so convert them to numerical when called from execute
-        $trace = debug_backtrace(\DEBUG_BACKTRACE_PROVIDE_OBJECT | \DEBUG_BACKTRACE_IGNORE_ARGS, 10);
-        $calledFromExecute = false;
-        foreach ($trace as $frame) {
-            if (($frame['object'] ?? null) === $this) {
-                if (($frame['function'] ?? null) === 'render') {
-                    continue;
-                } elseif (($frame['function'] ?? null) === 'execute') {
-                    $calledFromExecute = true;
-                }
-            }
-
-            break;
-        }
-
-        if ($calledFromExecute) {
-            $numParams = [];
-            $i = 0;
-            $j = 0;
-            $sql = preg_replace_callback(
-                '~(?:\'(?:\'\'|\\\\\'|[^\'])*\')?+\K(?:\?|:\w+)~s',
-                function ($matches) use ($params, &$numParams, &$i, &$j) {
-                    $numParams[++$i] = $params[$matches[0] === '?' ? ++$j : $matches[0]];
-
-                    return '?';
-                },
-                $sql
-            );
-            $params = $numParams;
-        }
-
         return [$sql, $params];
     }
+
+    protected function hasNativeNamedParamSupport(): bool
+    {
+        return false;
+    }
 }

src/Persistence/Sql/Mysql/Expression.php

@@ -8,5 +8,7 @@
 
 class Expression extends BaseExpression
 {
+    use ExpressionTrait;
+
     protected $escape_char = '`';
 }

src/Persistence/Sql/Mysql/ExpressionTrait.php

@@ -0,0 +1,22 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Atk4\Data\Persistence\Sql\Mysql;
+
+use Atk4\Data\Persistence\Sql\Connection as SqlConnection;
+
+trait ExpressionTrait
+{
+    protected function hasNativeNamedParamSupport(): bool
+    {
+        // TODO use Connection::getNativeConnection() once only DBAL 3.3+ is supported
+        // https://github.com/doctrine/dbal/pull/5037
+        $dbalConnection = $this->connection->connection();
+        $nativeConnection = \Closure::bind(function () use ($dbalConnection) {
+            return SqlConnection::getDriverFromDbalDriverConnection($dbalConnection->getWrappedConnection());
+        }, null, SqlConnection::class)();
+
+        return !$nativeConnection instanceof \mysqli;
+    }
+}

src/Persistence/Sql/Mysql/Query.php

@@ -8,6 +8,8 @@
 
 class Query extends BaseQuery
 {
+    use ExpressionTrait;
+
     protected $escape_char = '`';
 
     protected $expression_class = Expression::class;
@@ -16,6 +18,6 @@ class Query extends BaseQuery
 
     public function groupConcat($field, string $delimiter = ',')
     {
-        return $this->expr('group_concat({} separator [])', [$field, $delimiter]);
+        return $this->expr('group_concat({} separator \'' . str_replace('\'', '\'\'', $delimiter) . '\')', [$field]);
     }
 }

tests/Persistence/Sql/ConnectionTest.php

@@ -90,6 +90,16 @@ public function testDsnNormalize(): void
         // with port number as DSN, leave port as :port
         $dsn = Connection::normalizeDsn('mysql:host=localhost:1234;dbname=db');
         $this->assertSame(['driver' => 'pdo_mysql', 'host' => 'localhost', 'dbname' => 'db', 'port' => '1234'], $dsn);
+
+        // full PDO and native driver names
+        $dsn = Connection::normalizeDsn('pdo-mysql://root:pass@localhost/db');
+        $this->assertSame(['driver' => 'pdo_mysql', 'host' => 'localhost', 'user' => 'root', 'password' => 'pass', 'dbname' => 'db'], $dsn);
+
+        $dsn = Connection::normalizeDsn('pdo_mysql:host=localhost;dbname=db', 'root', 'pass');
+        $this->assertSame(['driver' => 'pdo_mysql', 'host' => 'localhost', 'dbname' => 'db', 'user' => 'root', 'password' => 'pass'], $dsn);
+
+        $dsn = Connection::normalizeDsn('mysqli://root:pass@localhost/db');
+        $this->assertSame(['driver' => 'mysqli', 'host' => 'localhost', 'user' => 'root', 'password' => 'pass', 'dbname' => 'db'], $dsn);
     }
 
     public function testConnectionRegistry(): void

tests/Persistence/Sql/QueryTest.php

@@ -1082,7 +1082,7 @@ public function testGroupConcatException(): void
     public function testGroupConcat(): void
     {
         $q = new Mysql\Query();
-        $this->assertSame('group_concat(`foo` separator :a)', $q->groupConcat('foo', '-')->render()[0]);
+        $this->assertSame('group_concat(`foo` separator \'-\')', $q->groupConcat('foo', '-')->render()[0]);
 
         $q = new Oracle\Query();
         $this->assertSame('listagg("foo", :a) within group (order by "foo")', $q->groupConcat('foo', '-')->render()[0]);

tests/Persistence/Sql/RandomTest.php

@@ -88,7 +88,7 @@ public function _groupConcatTest(string $expected, Query $q): void
     public function testGroupConcat(): void
     {
         $this->_groupConcatTest(
-            'select `age`, group_concat(`name` separator :a) from `people` group by `age`',
+            'select `age`, group_concat(`name` separator \',\') from `people` group by `age`',
             new Mysql\Query()
         );
 

tests/Persistence/Sql/WithDb/SelectTest.php

@@ -249,6 +249,11 @@ public function testExecuteException(): void
         } catch (ExecuteException $e) {
             if ($this->getDatabasePlatform() instanceof MySQLPlatform) {
                 $expectedErrorCode = 1146; // SQLSTATE[42S02]: Base table or view not found: 1146 Table 'non_existing_table' doesn't exist
+
+                $dummyExpr = $this->c->expr();
+                if (Connection::isComposerDbal2x() && !\Closure::bind(fn () => $dummyExpr->hasNativeNamedParamSupport(), null, \Atk4\Data\Persistence\Sql\Expression::class)()) {
+                    $this->markTestIncomplete('DBAL 2.x with mysqli driver does not set exception code');
+                }
             } elseif ($this->getDatabasePlatform() instanceof PostgreSQLPlatform) {
                 $expectedErrorCode = 7; // SQLSTATE[42P01]: Undefined table: 7 ERROR: relation "non_existing_table" does not exist
             } elseif ($this->getDatabasePlatform() instanceof SQLServerPlatform) {