Removed the logic of using BLOB columns for BINARY type of fields

morozov · May 31, 2019 · fb4476c · fb4476c
1 parent 0006e9e
commit fb4476c
Show file tree

Hide file tree

Showing 12 changed files with 63 additions and 148 deletions.
diff --git a/UPGRADE.md b/UPGRADE.md
@@ -102,6 +102,12 @@ The Drizzle project is abandoned and is therefore not supported by Doctrine DBAL
 - `Configuration::getSQLLogger()` does not return `null` anymore, but a `NullLogger` implementation.
 - `Configuration::setSQLLogger()` does not allow `null` anymore.
 
+## BC BREAK: Changes to handling binary fields
+
+- Binary fields whose length exceeds the maximum field size on a given platform are no longer represented as `BLOB`s.
+  Use binary fields of a size which fits all target platforms, or use blob explicitly instead.
+- Binary fields are no longer represented as streams in PHP. They are represented as strings.
+
 # Upgrade to 2.10
 
 ## Deprecated `Doctrine\DBAL\Driver::getName()`

diff --git a/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php b/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php
@@ -29,7 +29,6 @@
 use Doctrine\DBAL\Types\Type;
 use InvalidArgumentException;
 use UnexpectedValueException;
-use const E_USER_DEPRECATED;
 use function addcslashes;
 use function array_map;
 use function array_merge;
@@ -55,7 +54,6 @@
 use function strpos;
 use function strtolower;
 use function strtoupper;
-use function trigger_error;
 
 /**
  * Base class for all DatabasePlatforms. The DatabasePlatforms are the central
@@ -283,20 +281,6 @@ public function getBinaryTypeDeclarationSQL(array $field)
 
         $fixed = $field['fixed'] ?? false;
 
-        $maxLength = $this->getBinaryMaxLength();
-
-        if ($field['length'] > $maxLength) {
-            if ($maxLength > 0) {
-                @trigger_error(sprintf(
-                    'Binary field length %d is greater than supported by the platform (%d). Reduce the field length or use a BLOB field instead.',
-                    $field['length'],
-                    $maxLength
-                ), E_USER_DEPRECATED);
-            }
-
-            return $this->getBlobTypeDeclarationSQL($field);
-        }
-
         return $this->getBinaryTypeDeclarationSQLSnippet($field['length'], $fixed);
     }
 

diff --git a/lib/Doctrine/DBAL/Types/BinaryType.php b/lib/Doctrine/DBAL/Types/BinaryType.php
@@ -4,12 +4,9 @@
 
 use Doctrine\DBAL\ParameterType;
 use Doctrine\DBAL\Platforms\AbstractPlatform;
-use function assert;
-use function fopen;
-use function fseek;
-use function fwrite;
 use function is_resource;
 use function is_string;
+use function stream_get_contents;
 
 /**
  * Type that maps ab SQL BINARY/VARBINARY to a PHP resource stream.
@@ -33,15 +30,11 @@ public function convertToPHPValue($value, AbstractPlatform $platform)
             return null;
         }
 
-        if (is_string($value)) {
-            $fp = fopen('php://temp', 'rb+');
-            assert(is_resource($fp));
-            fwrite($fp, $value);
-            fseek($fp, 0);
-            $value = $fp;
+        if (is_resource($value)) {
+            $value = stream_get_contents($value);
         }
 
-        if (! is_resource($value)) {
+        if (! is_string($value)) {
             throw ConversionException::conversionFailed($value, Types::BINARY);
         }
 

diff --git a/tests/Doctrine/Tests/DBAL/Functional/Types/BinaryTest.php b/tests/Doctrine/Tests/DBAL/Functional/Types/BinaryTest.php
@@ -8,11 +8,10 @@
 use Doctrine\DBAL\Driver\PDOOracle\Driver as PDOOracleDriver;
 use Doctrine\DBAL\ParameterType;
 use Doctrine\DBAL\Schema\Table;
+use Doctrine\DBAL\Types\Type;
 use Doctrine\Tests\DbalFunctionalTestCase;
-use function is_resource;
 use function random_bytes;
 use function str_replace;
-use function stream_get_contents;
 
 class BinaryTest extends DbalFunctionalTestCase
 {
@@ -82,14 +81,6 @@ private function select(string $id)
             [ParameterType::BINARY]
         );
 
-        // Currently, `BinaryType` mistakenly converts string values fetched from the DB to a stream.
-        // It should be the opposite. Streams should be used to represent large objects, not binary
-        // strings. The confusion comes from the PostgreSQL's type system where binary strings and
-        // large objects are represented by the same BYTEA type
-        if (is_resource($value)) {
-            $value = stream_get_contents($value);
-        }
-
-        return $value;
+        return Type::getType('binary')->convertToPHPValue($value, $this->connection->getDatabasePlatform());
     }
 }
diff --git a/tests/Doctrine/Tests/DBAL/Platforms/AbstractMySQLPlatformTestCase.php b/tests/Doctrine/Tests/DBAL/Platforms/AbstractMySQLPlatformTestCase.php
@@ -538,27 +538,21 @@ public function testReturnsBinaryTypeDeclarationSQL() : void
         self::assertSame('VARBINARY(255)', $this->platform->getBinaryTypeDeclarationSQL([]));
         self::assertSame('VARBINARY(255)', $this->platform->getBinaryTypeDeclarationSQL(['length' => 0]));
         self::assertSame('VARBINARY(65535)', $this->platform->getBinaryTypeDeclarationSQL(['length' => 65535]));
+        self::assertSame('VARBINARY(65536)', $this->platform->getBinaryTypeDeclarationSQL(['length' => 65536]));
 
         self::assertSame('BINARY(255)', $this->platform->getBinaryTypeDeclarationSQL(['fixed' => true]));
-        self::assertSame('BINARY(255)', $this->platform->getBinaryTypeDeclarationSQL(['fixed' => true, 'length' => 0]));
-        self::assertSame('BINARY(65535)', $this->platform->getBinaryTypeDeclarationSQL(['fixed' => true, 'length' => 65535]));
-    }
-
-    /**
-     * @group legacy
-     * @expectedDeprecation Binary field length 65536 is greater than supported by the platform (65535). Reduce the field length or use a BLOB field instead.
-     * @expectedDeprecation Binary field length 16777215 is greater than supported by the platform (65535). Reduce the field length or use a BLOB field instead.
-     * @expectedDeprecation Binary field length 16777216 is greater than supported by the platform (65535). Reduce the field length or use a BLOB field instead.
-     */
-    public function testReturnsBinaryTypeLongerThanMaxDeclarationSQL() : void
-    {
-        self::assertSame('MEDIUMBLOB', $this->platform->getBinaryTypeDeclarationSQL(['length' => 65536]));
-        self::assertSame('MEDIUMBLOB', $this->platform->getBinaryTypeDeclarationSQL(['length' => 16777215]));
-        self::assertSame('LONGBLOB', $this->platform->getBinaryTypeDeclarationSQL(['length' => 16777216]));
-
-        self::assertSame('MEDIUMBLOB', $this->platform->getBinaryTypeDeclarationSQL(['fixed' => true, 'length' => 65536]));
-        self::assertSame('MEDIUMBLOB', $this->platform->getBinaryTypeDeclarationSQL(['fixed' => true, 'length' => 16777215]));
-        self::assertSame('LONGBLOB', $this->platform->getBinaryTypeDeclarationSQL(['fixed' => true, 'length' => 16777216]));
+        self::assertSame('BINARY(255)', $this->platform->getBinaryTypeDeclarationSQL([
+            'fixed' => true,
+            'length' => 0,
+        ]));
+        self::assertSame('BINARY(65535)', $this->platform->getBinaryTypeDeclarationSQL([
+            'fixed' => true,
+            'length' => 65535,
+        ]));
+        self::assertSame('BINARY(65536)', $this->platform->getBinaryTypeDeclarationSQL([
+            'fixed' => true,
+            'length' => 65536,
+        ]));
     }
 
     public function testDoesNotPropagateForeignKeyCreationForNonSupportingEngines() : void

diff --git a/tests/Doctrine/Tests/DBAL/Platforms/AbstractPlatformTestCase.php b/tests/Doctrine/Tests/DBAL/Platforms/AbstractPlatformTestCase.php
@@ -862,11 +862,6 @@ public function testReturnsBinaryTypeDeclarationSQL() : void
         $this->platform->getBinaryTypeDeclarationSQL([]);
     }
 
-    public function testReturnsBinaryTypeLongerThanMaxDeclarationSQL() : void
-    {
-        $this->markTestSkipped('Not applicable to the platform');
-    }
-
     /**
      * @group DBAL-553
      */

diff --git a/tests/Doctrine/Tests/DBAL/Platforms/AbstractSQLServerPlatformTestCase.php b/tests/Doctrine/Tests/DBAL/Platforms/AbstractSQLServerPlatformTestCase.php
@@ -1008,18 +1008,14 @@ public function testReturnsBinaryTypeDeclarationSQL() : void
         self::assertSame('VARBINARY(8000)', $this->platform->getBinaryTypeDeclarationSQL(['length' => 8000]));
 
         self::assertSame('BINARY(255)', $this->platform->getBinaryTypeDeclarationSQL(['fixed' => true]));
-        self::assertSame('BINARY(255)', $this->platform->getBinaryTypeDeclarationSQL(['fixed' => true, 'length' => 0]));
-        self::assertSame('BINARY(8000)', $this->platform->getBinaryTypeDeclarationSQL(['fixed' => true, 'length' => 8000]));
-    }
-
-    /**
-     * @group legacy
-     * @expectedDeprecation Binary field length 8001 is greater than supported by the platform (8000). Reduce the field length or use a BLOB field instead.
-     */
-    public function testReturnsBinaryTypeLongerThanMaxDeclarationSQL() : void
-    {
-        self::assertSame('VARBINARY(MAX)', $this->platform->getBinaryTypeDeclarationSQL(['length' => 8001]));
-        self::assertSame('VARBINARY(MAX)', $this->platform->getBinaryTypeDeclarationSQL(['fixed' => true, 'length' => 8001]));
+        self::assertSame('BINARY(255)', $this->platform->getBinaryTypeDeclarationSQL([
+            'fixed' => true,
+            'length' => 0,
+        ]));
+        self::assertSame('BINARY(8000)', $this->platform->getBinaryTypeDeclarationSQL([
+            'fixed' => true,
+            'length' => 8000,
+        ]));
     }
 
     /**

diff --git a/tests/Doctrine/Tests/DBAL/Platforms/DB2PlatformTest.php b/tests/Doctrine/Tests/DBAL/Platforms/DB2PlatformTest.php
@@ -446,16 +446,6 @@ public function testReturnsBinaryTypeDeclarationSQL() : void
         self::assertSame('CHAR(254) FOR BIT DATA', $this->platform->getBinaryTypeDeclarationSQL(['fixed' => true, 'length' => 0]));
     }
 
-    /**
-     * @group legacy
-     * @expectedDeprecation Binary field length 32705 is greater than supported by the platform (32704). Reduce the field length or use a BLOB field instead.
-     */
-    public function testReturnsBinaryTypeLongerThanMaxDeclarationSQL() : void
-    {
-        self::assertSame('BLOB(1M)', $this->platform->getBinaryTypeDeclarationSQL(['length' => 32705]));
-        self::assertSame('BLOB(1M)', $this->platform->getBinaryTypeDeclarationSQL(['fixed' => true, 'length' => 32705]));
-    }
-
     /**
      * {@inheritDoc}
      *

diff --git a/tests/Doctrine/Tests/DBAL/Platforms/OraclePlatformTest.php b/tests/Doctrine/Tests/DBAL/Platforms/OraclePlatformTest.php
@@ -513,18 +513,14 @@ public function testReturnsBinaryTypeDeclarationSQL() : void
         self::assertSame('RAW(2000)', $this->platform->getBinaryTypeDeclarationSQL(['length' => 2000]));
 
         self::assertSame('RAW(255)', $this->platform->getBinaryTypeDeclarationSQL(['fixed' => true]));
-        self::assertSame('RAW(2000)', $this->platform->getBinaryTypeDeclarationSQL(['fixed' => true, 'length' => 0]));
-        self::assertSame('RAW(2000)', $this->platform->getBinaryTypeDeclarationSQL(['fixed' => true, 'length' => 2000]));
-    }
-
-    /**
-     * @group legacy
-     * @expectedDeprecation Binary field length 2001 is greater than supported by the platform (2000). Reduce the field length or use a BLOB field instead.
-     */
-    public function testReturnsBinaryTypeLongerThanMaxDeclarationSQL() : void
-    {
-        self::assertSame('BLOB', $this->platform->getBinaryTypeDeclarationSQL(['length' => 2001]));
-        self::assertSame('BLOB', $this->platform->getBinaryTypeDeclarationSQL(['fixed' => true, 'length' => 2001]));
+        self::assertSame('RAW(2000)', $this->platform->getBinaryTypeDeclarationSQL([
+            'fixed' => true,
+            'length' => 0,
+        ]));
+        self::assertSame('RAW(2000)', $this->platform->getBinaryTypeDeclarationSQL([
+            'fixed' => true,
+            'length' => 2000,
+        ]));
     }
 
     public function testDoesNotPropagateUnnecessaryTableAlterationOnBinaryType() : void

diff --git a/tests/Doctrine/Tests/DBAL/Platforms/SQLAnywherePlatformTest.php b/tests/Doctrine/Tests/DBAL/Platforms/SQLAnywherePlatformTest.php
@@ -966,18 +966,14 @@ public function testReturnsBinaryTypeDeclarationSQL() : void
         self::assertSame('VARBINARY(32767)', $this->platform->getBinaryTypeDeclarationSQL(['length' => 32767]));
 
         self::assertSame('BINARY(1)', $this->platform->getBinaryTypeDeclarationSQL(['fixed' => true]));
-        self::assertSame('BINARY(1)', $this->platform->getBinaryTypeDeclarationSQL(['fixed' => true, 'length' => 0]));
-        self::assertSame('BINARY(32767)', $this->platform->getBinaryTypeDeclarationSQL(['fixed' => true, 'length' => 32767]));
-    }
-
-    /**
-     * @group legacy
-     * @expectedDeprecation Binary field length 32768 is greater than supported by the platform (32767). Reduce the field length or use a BLOB field instead.
-     */
-    public function testReturnsBinaryTypeLongerThanMaxDeclarationSQL() : void
-    {
-        self::assertSame('LONG BINARY', $this->platform->getBinaryTypeDeclarationSQL(['length' => 32768]));
-        self::assertSame('LONG BINARY', $this->platform->getBinaryTypeDeclarationSQL(['fixed' => true, 'length' => 32768]));
+        self::assertSame('BINARY(1)', $this->platform->getBinaryTypeDeclarationSQL([
+            'fixed' => true,
+            'length' => 0,
+        ]));
+        self::assertSame('BINARY(32767)', $this->platform->getBinaryTypeDeclarationSQL([
+            'fixed' => true,
+            'length' => 32767,
+        ]));
     }
 
     /**

diff --git a/tests/Doctrine/Tests/DBAL/Types/BinaryTest.php b/tests/Doctrine/Tests/DBAL/Types/BinaryTest.php
@@ -10,9 +10,11 @@
 use Doctrine\DBAL\Types\Types;
 use Doctrine\Tests\DbalTestCase;
 use PHPUnit\Framework\MockObject\MockObject;
+use function array_map;
 use function base64_encode;
 use function fopen;
-use function stream_get_contents;
+use function implode;
+use function range;
 
 class BinaryTest extends DbalTestCase
 {
@@ -57,19 +59,26 @@ public function testBinaryNullConvertsToPHPValue() : void
 
     public function testBinaryStringConvertsToPHPValue() : void
     {
-        $databaseValue = 'binary string';
+        $databaseValue = $this->getBinaryString();
         $phpValue      = $this->type->convertToPHPValue($databaseValue, $this->platform);
 
-        self::assertIsResource($phpValue);
-        self::assertEquals($databaseValue, stream_get_contents($phpValue));
+        self::assertSame($databaseValue, $phpValue);
     }
 
     public function testBinaryResourceConvertsToPHPValue() : void
     {
         $databaseValue = fopen('data://text/plain;base64,' . base64_encode('binary string'), 'r');
         $phpValue      = $this->type->convertToPHPValue($databaseValue, $this->platform);
 
-        self::assertSame($databaseValue, $phpValue);
+        self::assertSame('binary string', $phpValue);
+    }
+
+    /**
+     * Creates a binary string containing all possible byte values.
+     */
+    private function getBinaryString() : string
+    {
+        return implode(array_map('chr', range(0, 255)));
     }
 
     /**

diff --git a/tests/Doctrine/Tests/DBAL/Types/BlobTest.php b/tests/Doctrine/Tests/DBAL/Types/BlobTest.php
@@ -7,10 +7,6 @@
 use Doctrine\DBAL\Types\Type;
 use Doctrine\Tests\DbalTestCase;
 use PHPUnit\Framework\MockObject\MockObject;
-use function base64_encode;
-use function chr;
-use function fopen;
-use function stream_get_contents;
 
 class BlobTest extends DbalTestCase
 {
@@ -33,35 +29,4 @@ public function testBlobNullConvertsToPHPValue() : void
     {
         self::assertNull($this->type->convertToPHPValue(null, $this->platform));
     }
-
-    public function testBinaryStringConvertsToPHPValue() : void
-    {
-        $databaseValue = $this->getBinaryString();
-        $phpValue      = $this->type->convertToPHPValue($databaseValue, $this->platform);
-
-        self::assertIsResource($phpValue);
-        self::assertSame($databaseValue, stream_get_contents($phpValue));
-    }
-
-    public function testBinaryResourceConvertsToPHPValue() : void
-    {
-        $databaseValue = fopen('data://text/plain;base64,' . base64_encode($this->getBinaryString()), 'r');
-        $phpValue      = $this->type->convertToPHPValue($databaseValue, $this->platform);
-
-        self::assertSame($databaseValue, $phpValue);
-    }
-
-    /**
-     * Creates a binary string containing all possible byte values.
-     */
-    private function getBinaryString() : string
-    {
-        $string = '';
-
-        for ($i = 0; $i < 256; $i++) {
-            $string .= chr($i);
-        }
-
-        return $string;
-    }
 }