Add binary/blob column support for MSSQL and Oracle

.github/workflows/test-unit.yml

@@ -48,7 +48,7 @@ jobs:
         run: |
           if [ "${{ matrix.type }}" != "Phpunit" ] && [ "${{ matrix.type }}" != "StaticAnalysis" ]; then composer remove --no-interaction --no-update phpunit/phpunit johnkary/phpunit-speedtrap --dev; fi
           if [ "${{ matrix.type }}" != "CodingStyle" ]; then composer remove --no-interaction --no-update friendsofphp/php-cs-fixer --dev && composer --no-interaction --no-update require jdorn/sql-formatter; fi
-          if [ "${{ matrix.type }}" != "StaticAnalysis" ]; then composer remove --no-interaction --no-update phpstan/* --dev; fi
+          if [ "${{ matrix.type }}" != "StaticAnalysis" ]; then composer remove --no-interaction --no-update phpstan/\* --dev; fi
           composer update --ansi --prefer-dist --no-interaction --no-progress --optimize-autoloader
 
       - name: "Run tests: SQLite (only for Phpunit)"
@@ -144,11 +144,11 @@ jobs:
         run: |
           if [ "${{ matrix.type }}" != "Phpunit" ] && [ "${{ matrix.type }}" != "Phpunit Lowest" ] && [ "${{ matrix.type }}" != "Phpunit Burn" ]; then composer remove --no-interaction --no-update phpunit/phpunit johnkary/phpunit-speedtrap --dev; fi
           if [ "${{ matrix.type }}" != "CodingStyle" ]; then composer remove --no-interaction --no-update friendsofphp/php-cs-fixer --dev && composer --no-update --ansi --prefer-dist --no-interaction --no-progress require jdorn/sql-formatter; fi
-          if [ "${{ matrix.type }}" != "StaticAnalysis" ]; then composer remove --no-interaction --no-update phpstan/* --dev; fi
+          if [ "${{ matrix.type }}" != "StaticAnalysis" ]; then composer remove --no-interaction --no-update phpstan/\* --dev; fi
           if [ -n "$LOG_COVERAGE" ]; then composer require --no-interaction --no-update phpunit/phpcov; fi
           composer update --ansi --prefer-dist --no-interaction --no-progress --optimize-autoloader
-          if [ "${{ matrix.type }}" == "Phpunit Lowest" ]; then composer update --ansi --prefer-dist --prefer-lowest --prefer-stable --no-interaction --no-progress --optimize-autoloader; fi
-          if [ "${{ matrix.type }}" == "Phpunit Burn" ]; then sed -i 's~ *public function runBare(): void~public function runBare(): void { gc_collect_cycles(); gc_collect_cycles(); $memDiffs = array_fill(0, '"$(if [ \"$GITHUB_EVENT_NAME\" == \"schedule\" ]; then echo 64; else echo 4; fi)"', 0); for ($i = -1; $i < count($memDiffs); ++$i) { $this->_runBare(); gc_collect_cycles(); gc_collect_cycles(); $mem = memory_get_usage(); if ($i !== -1) { $memDiffs[$i] = $mem - $memPrev; } $memPrev = $mem; rsort($memDiffs); if (array_sum($memDiffs) >= 4096 * 1024 || $memDiffs[2] > 0) { $this->onNotSuccessfulTest(new AssertionFailedError( "Memory leak detected! (" . implode(" + ", array_map(fn ($v) => number_format($v / 1024, 3, ".", " "), array_filter($memDiffs))) . " KB, " . ($i + 2) . " iterations)" )); } } } private function _runBare(): void~' vendor/phpunit/phpunit/src/Framework/TestCase.php && cat vendor/phpunit/phpunit/src/Framework/TestCase.php | grep '_runBare('; fi
+          if [ "${{ matrix.type }}" = "Phpunit Lowest" ]; then composer update --ansi --prefer-dist --prefer-lowest --prefer-stable --no-interaction --no-progress --optimize-autoloader; fi
+          if [ "${{ matrix.type }}" = "Phpunit Burn" ]; then sed -i 's~ *public function runBare(): void~public function runBare(): void { gc_collect_cycles(); gc_collect_cycles(); $memDiffs = array_fill(0, '"$(if [ \"$GITHUB_EVENT_NAME\" == \"schedule\" ]; then echo 64; else echo 4; fi)"', 0); for ($i = -1; $i < count($memDiffs); ++$i) { $this->_runBare(); gc_collect_cycles(); gc_collect_cycles(); $mem = memory_get_usage(); if ($i !== -1) { $memDiffs[$i] = $mem - $memPrev; } $memPrev = $mem; rsort($memDiffs); if (array_sum($memDiffs) >= 4096 * 1024 || $memDiffs[2] > 0) { $this->onNotSuccessfulTest(new AssertionFailedError( "Memory leak detected! (" . implode(" + ", array_map(fn ($v) => number_format($v / 1024, 3, ".", " "), array_filter($memDiffs))) . " KB, " . ($i + 2) . " iterations)" )); } } } private function _runBare(): void~' vendor/phpunit/phpunit/src/Framework/TestCase.php && cat vendor/phpunit/phpunit/src/Framework/TestCase.php | grep '_runBare('; fi
 
       - name: Init
         run: |
@@ -201,9 +201,10 @@ jobs:
       - name: "Run tests: Oracle (only for coverage or cron)"
         if: env.LOG_COVERAGE || github.event_name == 'schedule'
         env:
-          DB_DSN: "oci:dbname=oracle/xe;charset=UTF8"
+          DB_DSN: "oci:dbname=oracle/xe"
           DB_USER: system
           DB_PASSWORD: oracle
+          NLS_LANG: AMERICAN_AMERICA.AL32UTF8
         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 \
           || 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

bootstrap-types.php

@@ -39,6 +39,11 @@ public function convertToPHPValue($value, AbstractPlatform $platform): ?float
 
         return $v === null ? null : (float) $v;
     }
+
+    public function requiresSQLCommentHint(AbstractPlatform $platform)
+    {
+        return true;
+    }
 }
 
 DbalTypes\Type::addType(Types::MONEY, MoneyType::class);

phpstan.neon.dist

@@ -31,7 +31,7 @@ parameters:
             message: '~^(Call to an undefined method Doctrine\\DBAL\\Driver\\Connection::getWrappedConnection\(\)\.|Caught class Doctrine\\DBAL\\DBALException not found\.|Call to static method notSupported\(\) on an unknown class Doctrine\\DBAL\\DBALException\.|Access to an undefined property Doctrine\\DBAL\\Driver\\PDO\\Connection::\$connection\.|Method Atk4\\Data\\Persistence\\Sql\\Expression::execute\(\) should return Doctrine\\DBAL\\Result\|PDOStatement but returns bool\.|Class Doctrine\\DBAL\\Platforms\\MySqlPlatform referenced with incorrect case: Doctrine\\DBAL\\Platforms\\MySQLPlatform\.)$~'
             path: '*'
             # count for DBAL 3.x matched in "src/Persistence/GenericPlatform.php" file
-            count: 10
+            count: 11
 
         # TODO these rules are generated, this ignores should be fixed in the code
         # for src/Schema/TestCase.php

src/Persistence.php

@@ -45,20 +45,11 @@ public static function connect($dsn, string $user = null, string $password = nul
         $dsn = \Atk4\Data\Persistence\Sql\Connection::normalizeDsn($dsn, $user, $password);
 
         switch ($dsn['driverSchema']) {
+            case 'sqlite':
             case 'mysql':
-            case 'oci':
-            case 'oci12':
-                // Omitting UTF8 is always a bad problem, so unless it's specified we will do that
-                // to prevent nasty problems. This is un-tested on other databases, so moving it here.
-                // It gives problem with sqlite
-                if (strpos($dsn['dsn'], ';charset=') === false) {
-                    $dsn['dsn'] .= ';charset=utf8mb4';
-                }
-
-                // no break
             case 'pgsql':
             case 'sqlsrv':
-            case 'sqlite':
+            case 'oci':
                 $db = new \Atk4\Data\Persistence\Sql($dsn['dsn'], $dsn['user'], $dsn['pass'], $args);
 
                 return $db;

src/Persistence/GenericPlatform.php

@@ -6,14 +6,16 @@
 
 use Doctrine\DBAL\Exception as DbalException;
 use Doctrine\DBAL\Platforms;
+use Mvorisek\Atk4\Hintable\Phpstan\PhpstanUtil;
 
 class GenericPlatform extends Platforms\AbstractPlatform
 {
     private function createNotSupportedException(): \Exception // DbalException once DBAL 2.x support is dropped
     {
         if (\Atk4\Data\Persistence\Sql\Connection::isComposerDbal2x()) {
             // hack for PHPStan, keep ignored error count for DBAL 2.x and DBAL 3.x the same
-            if (\PHP_MAJOR_VERSION === 0) {
+            if (PhpstanUtil::alwaysFalseAnalyseOnly()) {
+                \Doctrine\DBAL\DBALException::notSupported('SQL');
                 \Doctrine\DBAL\DBALException::notSupported('SQL');
                 \Doctrine\DBAL\DBALException::notSupported('SQL');
                 \Doctrine\DBAL\DBALException::notSupported('SQL');

src/Persistence/Sql.php

@@ -20,6 +20,8 @@
 
 class Sql extends Persistence
 {
+    use Sql\BinaryTypeCompatibilityTypecastTrait;
+
     /** @const string */
     public const HOOK_INIT_SELECT_QUERY = self::class . '@initSelectQuery';
     /** @const string */

src/Persistence/Sql/BinaryTypeCompatibilityTypecastTrait.php

@@ -0,0 +1,84 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Atk4\Data\Persistence\Sql;
+
+use Atk4\Data\Exception;
+use Atk4\Data\Field;
+use Doctrine\DBAL\Platforms\OraclePlatform;
+use Doctrine\DBAL\Platforms\PostgreSQL94Platform;
+use Doctrine\DBAL\Platforms\SQLServer2012Platform;
+use Doctrine\DBAL\Types\Type;
+
+trait BinaryTypeCompatibilityTypecastTrait
+{
+    private function binaryTypeValueGetPrefixConst(): string
+    {
+        return 'atk__binary__u5f8mzx4vsm8g2c9__';
+    }
+
+    private function binaryTypeValueEncode(string $value): string
+    {
+        $hex = bin2hex($value);
+
+        return $this->binaryTypeValueGetPrefixConst() . hash('crc32b', $hex) . $hex;
+    }
+
+    private function binaryTypeValueIsEncoded(string $value): bool
+    {
+        return str_starts_with($value, $this->binaryTypeValueGetPrefixConst());
+    }
+
+    private function binaryTypeValueDecode(string $value): string
+    {
+        if (!$this->binaryTypeValueIsEncoded($value)) {
+            throw new Exception('Unexpected unencoded binary value');
+        }
+
+        $hexCrc = substr($value, strlen($this->binaryTypeValueGetPrefixConst()), 8);
+        $hex = substr($value, strlen($this->binaryTypeValueGetPrefixConst()) + 8);
+        if ((strlen($hex) % 2) !== 0 || $hexCrc !== hash('crc32b', $hex)) {
+            throw new Exception('Unexpected binary value crc');
+        }
+
+        return hex2bin($hex);
+    }
+
+    private function binaryTypeIsEncodeNeeded(Type $type): bool
+    {
+        // TODO PostgreSQL tests fail without binary compatibility typecast
+        $platform = $this->getDatabasePlatform();
+        if ($platform instanceof PostgreSQL94Platform
+            || $platform instanceof SQLServer2012Platform
+            || $platform instanceof OraclePlatform) {
+            if (in_array($type->getName(), ['binary', 'blob'], true)) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    public function typecastSaveField(Field $field, $value)
+    {
+        $value = parent::typecastSaveField($field, $value);
+
+        if ($value !== null && $this->binaryTypeIsEncodeNeeded($field->getTypeObject())) {
+            $value = $this->binaryTypeValueEncode($value);
+        }
+
+        return $value;
+    }
+
+    public function typecastLoadField(Field $field, $value)
+    {
+        $value = parent::typecastLoadField($field, $value);
+
+        if ($value !== null && $this->binaryTypeIsEncodeNeeded($field->getTypeObject())) {
+            $value = $this->binaryTypeValueDecode($value);
+        }
+
+        return $value;
+    }
+}

src/Persistence/Sql/Connection.php

@@ -212,6 +212,16 @@ protected static function connectDbalConnection(array $dsn)
         if (isset($dsn['pdo'])) {
             $pdo = $dsn['pdo'];
         } else {
+            $enforceCharset = [
+                'mysql' => 'utf8mb4',
+                'oci' => 'AL32UTF8',
+            ][$dsn['driverSchema']] ?? null;
+
+            if ($enforceCharset !== null) {
+                $dsn['dsn'] = preg_replace('~; *charset=[^;]+~i', '', $dsn['dsn'])
+                    . ';charset=' . $enforceCharset;
+            }
+
             $pdo = new \PDO($dsn['dsn'], $dsn['user'], $dsn['pass']);
         }
 

src/Persistence/Sql/Expression.php

@@ -7,6 +7,7 @@
 use Atk4\Core\WarnDynamicPropertyTrait;
 use Doctrine\DBAL\Connection as DbalConnection;
 use Doctrine\DBAL\Exception as DbalException;
+use Doctrine\DBAL\ParameterType;
 use Doctrine\DBAL\Platforms\OraclePlatform;
 use Doctrine\DBAL\Platforms\PostgreSQL94Platform;
 use Doctrine\DBAL\Result as DbalResult;
@@ -512,19 +513,21 @@ public function execute(object $connection = null): object
 
                 foreach ($this->params as $key => $val) {
                     if (is_int($val)) {
-                        $type = \PDO::PARAM_INT;
+                        $type = ParameterType::INTEGER;
                     } elseif (is_bool($val)) {
                         if ($this->connection->getDatabasePlatform() instanceof PostgreSQL94Platform) {
-                            $type = \PDO::PARAM_STR;
+                            $type = ParameterType::STRING;
                             $val = $val ? '1' : '0';
                         } else {
-                            $type = \PDO::PARAM_INT;
+                            $type = ParameterType::INTEGER;
                             $val = $val ? 1 : 0;
                         }
                     } elseif ($val === null) {
-                        $type = \PDO::PARAM_NULL;
-                    } elseif (is_string($val) || is_float($val)) {
-                        $type = \PDO::PARAM_STR;
+                        $type = ParameterType::NULL;
+                    } elseif (is_float($val)) {
+                        $type = ParameterType::STRING;
+                    } elseif (is_string($val)) {
+                        $type = ParameterType::STRING;
                     } elseif (is_resource($val)) {
                         throw new Exception('Resource type is not supported, set value as string instead');
                     } else {
@@ -594,9 +597,11 @@ private function getCastValue($v): ?string
             return $v ? '1' : '0';
         }
 
-        // for Oracle CLOB/BLOB datatypes and PDO driver
-        if (is_resource($v) && get_resource_type($v) === 'stream'
-                && $this->connection->getDatabasePlatform() instanceof OraclePlatform) {
+        // for PostgreSQL/Oracle CLOB/BLOB datatypes and PDO driver
+        if (is_resource($v) && get_resource_type($v) === 'stream' && (
+            $this->connection->getDatabasePlatform() instanceof PostgreSQL94Platform
+            || $this->connection->getDatabasePlatform() instanceof OraclePlatform
+        )) {
             $v = stream_get_contents($v);
         }
 

src/Persistence/Sql/Mssql/ExpressionTrait.php

@@ -23,6 +23,14 @@ private function fixOpenEscapeChar(string $v): string
         return preg_replace('~(?:\'(?:\'\'|\\\\\'|[^\'])*\')?+\K\]([^\[\]\'"(){}]*?)\]~s', '[$1]', $v);
     }
 
+    private function _render(): string
+    {
+        // convert all SQL strings to NVARCHAR, eg 'text' to N'text'
+        return preg_replace_callback('~(^|.)(\'(?:\'\'|\\\\\'|[^\'])*\')~s', function ($matches) {
+            return $matches[1] . (!in_array($matches[1], ['N', '\'', '\\'], true) ? 'N' : '') . $matches[2];
+        }, parent::render());
+    }
+
     // {{{ MSSQL does not support named parameters, so convert them to numerical inside execute
 
     /** @var array|null */
@@ -51,7 +59,7 @@ function ($matches) use (&$numParams, &$i, &$j) {
 
                     return '?';
                 },
-                parent::render()
+                $this->_render()
             );
             $this->params = $numParams;
 
@@ -69,7 +77,7 @@ public function render(): string
             return $this->numQueryRender;
         }
 
-        return parent::render();
+        return $this->_render();
     }
 
     public function getDebugQuery(): string

src/Persistence/Sql/Mssql/PlatformTrait.php

@@ -6,6 +6,37 @@
 
 trait PlatformTrait
 {
+    // SQL Server database requires explicit conversion when using binary column,
+    // workaround by using a standard non-binary column with custom encoding/typecast
+
+    protected function getBinaryTypeDeclarationSQLSnippet($length, $fixed)
+    {
+        return $this->getVarcharTypeDeclarationSQLSnippet($length, $fixed);
+    }
+
+    public function getBlobTypeDeclarationSQL(array $column)
+    {
+        return $this->getClobTypeDeclarationSQL($column);
+    }
+
+    // remove once https://github.com/doctrine/dbal/pull/4987 is fixed
+    // and also $this->markDoctrineTypeCommented('text') below
+    public function getClobTypeDeclarationSQL(array $column)
+    {
+        $res = parent::getClobTypeDeclarationSQL($column);
+
+        return (str_starts_with($res, 'VARCHAR') ? 'N' : '') . $res;
+    }
+
+    protected function initializeCommentedDoctrineTypes()
+    {
+        parent::initializeCommentedDoctrineTypes();
+
+        $this->markDoctrineTypeCommented('binary');
+        $this->markDoctrineTypeCommented('blob');
+        $this->markDoctrineTypeCommented('text');
+    }
+
     // SQL Server DBAL platform has buggy identifier escaping, fix until fixed officially, see:
     // https://github.com/doctrine/dbal/pull/4360
 

src/Persistence/Sql/Oracle/PlatformTrait.php

@@ -10,6 +10,14 @@
 
 trait PlatformTrait
 {
+    // Oracle database requires explicit conversion when using binary column,
+    // workaround by using a standard non-binary column with custom encoding/typecast
+
+    protected function getBinaryTypeDeclarationSQLSnippet($length, $fixed)
+    {
+        return $this->getVarcharTypeDeclarationSQLSnippet($length, $fixed);
+    }
+
     // Oracle CLOB/BLOB has limited SQL support, see:
     // https://stackoverflow.com/questions/12980038/ora-00932-inconsistent-datatypes-expected-got-clob#12980560
     // fix this Oracle inconsistency by using VARCHAR/VARBINARY instead (but limited to 4000 bytes)
@@ -41,6 +49,15 @@ public function getBlobTypeDeclarationSQL(array $column)
         return $this->forwardTypeDeclarationSQL('getBinaryTypeDeclarationSQL', $column);
     }
 
+    protected function initializeCommentedDoctrineTypes()
+    {
+        parent::initializeCommentedDoctrineTypes();
+
+        $this->markDoctrineTypeCommented('binary');
+        $this->markDoctrineTypeCommented('text');
+        $this->markDoctrineTypeCommented('blob');
+    }
+
     // Oracle DBAL platform autoincrement implementation does not increment like
     // Sqlite or MySQL does, unify the behaviour