Merge pull request #9705 from morozov/remove-require-conversion

Remove support for Type::canRequireSQLConversion()
doctrine · May 1, 2022 · 04830b7 · 04830b7
2 parents 3043dcc + fa844b1
commit 04830b7
Show file tree

Hide file tree

Showing 16 changed files with 28 additions and 112 deletions.
diff --git a/UPGRADE.md b/UPGRADE.md
@@ -1,5 +1,13 @@
 # Upgrade to 3.0
 
+## BC BREAK: Remove support for `Type::canRequireSQLConversion()`
+
+This feature was deprecated in DBAL 3.3.0 and will be removed in DBAL 4.0.
+The value conversion methods are now called regardless of the type.
+
+The `MappingException::sqlConversionNotAllowedForIdentifiers()` method has been removed
+as no longer relevant.
+
 ## BC Break: Removed the `doctrine` binary.
 
 The documentation explains how the console tools can be bootstrapped for

diff --git a/docs/en/cookbook/advanced-field-value-conversion-using-custom-mapping-types.rst b/docs/en/cookbook/advanced-field-value-conversion-using-custom-mapping-types.rst
@@ -171,11 +171,6 @@ Now we're going to create the ``point`` type and implement all required methods.
             return $value;
         }
 
-        public function canRequireSQLConversion()
-        {
-            return true;
-        }
-
         public function convertToPHPValueSQL($sqlExpr, AbstractPlatform $platform)
         {
             return sprintf('AsText(%s)', $sqlExpr);

diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php
@@ -10,7 +10,6 @@
 use DateTime;
 use DateTimeImmutable;
 use Doctrine\DBAL\Platforms\AbstractPlatform;
-use Doctrine\DBAL\Types\Type;
 use Doctrine\DBAL\Types\Types;
 use Doctrine\Deprecations\Deprecation;
 use Doctrine\Instantiator\Instantiator;
@@ -92,7 +91,6 @@
  *      originalClass?: class-string,
  *      originalField?: string,
  *      quoted?: bool,
- *      requireSQLConversion?: bool,
  *      declared?: class-string,
  *      declaredField?: string,
  *      options?: array<string, mixed>
@@ -1537,14 +1535,6 @@ protected function validateAndCompleteFieldMapping(array $mapping): array
             }
         }
 
-        if (Type::hasType($mapping['type']) && Type::getType($mapping['type'])->canRequireSQLConversion()) {
-            if (isset($mapping['id']) && $mapping['id'] === true) {
-                 throw MappingException::sqlConversionNotAllowedForIdentifiers($this->name, $mapping['fieldName'], $mapping['type']);
-            }
-
-            $mapping['requireSQLConversion'] = true;
-        }
-
         if (isset($mapping['generated'])) {
             if (! in_array($mapping['generated'], [self::GENERATED_NEVER, self::GENERATED_INSERT, self::GENERATED_ALWAYS])) {
                 throw MappingException::invalidGeneratedMode($mapping['generated']);

diff --git a/lib/Doctrine/ORM/Mapping/MappingException.php b/lib/Doctrine/ORM/Mapping/MappingException.php
@@ -593,24 +593,6 @@ public static function cannotVersionIdField($className, $fieldName)
         ));
     }
 
-    /**
-     * @param string $className
-     * @param string $fieldName
-     * @param string $type
-     *
-     * @return MappingException
-     */
-    public static function sqlConversionNotAllowedForIdentifiers($className, $fieldName, $type)
-    {
-        return new self(sprintf(
-            "It is not possible to set id field '%s' to type '%s' in entity class '%s'. The type '%s' requires conversion SQL which is not allowed for identifiers.",
-            $fieldName,
-            $type,
-            $className,
-            $type
-        ));
-    }
-
     /**
      * @param string $className
      * @param string $columnName

diff --git a/lib/Doctrine/ORM/Persisters/Entity/AbstractEntityInheritancePersister.php b/lib/Doctrine/ORM/Persisters/Entity/AbstractEntityInheritancePersister.php
@@ -49,10 +49,8 @@ protected function getSelectColumnSQL(string $field, ClassMetadata $class, strin
 
         $this->currentPersisterContext->rsm->addFieldResult($alias, $columnAlias, $field, $class->name);
 
-        if (isset($fieldMapping['requireSQLConversion'])) {
-            $type = Type::getType($fieldMapping['type']);
-            $sql  = $type->convertToPHPValueSQL($sql, $this->platform);
-        }
+        $type = Type::getType($fieldMapping['type']);
+        $sql  = $type->convertToPHPValueSQL($sql, $this->platform);
 
         return $sql . ' AS ' . $columnAlias;
     }

diff --git a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php
@@ -401,7 +401,7 @@ final protected function updateTable(
                     $fieldName = $this->class->fieldNames[$columnName];
                     $column    = $this->quoteStrategy->getColumnName($fieldName, $this->class, $this->platform);
 
-                    if (isset($this->class->fieldMappings[$fieldName]['requireSQLConversion'])) {
+                    if (isset($this->class->fieldMappings[$fieldName])) {
                         $type        = Type::getType($this->columnTypes[$columnName]);
                         $placeholder = $type->convertToDatabaseValueSQL('?', $this->platform);
                     }
@@ -1348,7 +1348,7 @@ public function getInsertSQL(): string
             if (
                 isset($this->class->fieldNames[$column])
                 && isset($this->columnTypes[$this->class->fieldNames[$column]])
-                && isset($this->class->fieldMappings[$this->class->fieldNames[$column]]['requireSQLConversion'])
+                && isset($this->class->fieldMappings[$this->class->fieldNames[$column]])
             ) {
                 $type        = Type::getType($this->columnTypes[$this->class->fieldNames[$column]]);
                 $placeholder = $type->convertToDatabaseValueSQL('?', $this->platform);
@@ -1427,10 +1427,8 @@ protected function getSelectColumnSQL(string $field, ClassMetadata $class, strin
 
         $this->currentPersisterContext->rsm->addFieldResult($alias, $columnAlias, $field);
 
-        if (isset($fieldMapping['requireSQLConversion'])) {
-            $type = Type::getType($fieldMapping['type']);
-            $sql  = $type->convertToPHPValueSQL($sql, $this->platform);
-        }
+        $type = Type::getType($fieldMapping['type']);
+        $sql  = $type->convertToPHPValueSQL($sql, $this->platform);
 
         return $sql . ' AS ' . $columnAlias;
     }
@@ -1534,7 +1532,7 @@ public function getSelectConditionStatementSQL(string $field, mixed $value, ?arr
         foreach ($columns as $column) {
             $placeholder = '?';
 
-            if (isset($this->class->fieldMappings[$field]['requireSQLConversion'])) {
+            if (isset($this->class->fieldMappings[$field])) {
                 $type        = Type::getType($this->class->fieldMappings[$field]['type']);
                 $placeholder = $type->convertToDatabaseValueSQL($placeholder, $this->platform);
             }

diff --git a/lib/Doctrine/ORM/Query/ResultSetMappingBuilder.php b/lib/Doctrine/ORM/Query/ResultSetMappingBuilder.php
@@ -254,10 +254,8 @@ public function generateSelectClause(array $tableAliases = []): string
                 $classFieldMapping = $class->fieldMappings[$fieldName];
                 $columnSql         = $tableAlias . '.' . $classFieldMapping['columnName'];
 
-                if (isset($classFieldMapping['requireSQLConversion']) && $classFieldMapping['requireSQLConversion'] === true) {
-                    $type      = Type::getType($classFieldMapping['type']);
-                    $columnSql = $type->convertToPHPValueSQL($columnSql, $this->em->getConnection()->getDatabasePlatform());
-                }
+                $type      = Type::getType($classFieldMapping['type']);
+                $columnSql = $type->convertToPHPValueSQL($columnSql, $this->em->getConnection()->getDatabasePlatform());
 
                 $sql .= $columnSql;
             } elseif (isset($this->metaMappings[$columnName])) {

diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php
@@ -1347,10 +1347,8 @@ public function walkSelectExpression($selectExpression)
                 $columnAlias   = $this->getSQLColumnAlias($fieldMapping['columnName']);
                 $col           = $sqlTableAlias . '.' . $columnName;
 
-                if (isset($fieldMapping['requireSQLConversion'])) {
-                    $type = Type::getType($fieldMapping['type']);
-                    $col  = $type->convertToPHPValueSQL($col, $this->conn->getDatabasePlatform());
-                }
+                $type = Type::getType($fieldMapping['type']);
+                $col  = $type->convertToPHPValueSQL($col, $this->conn->getDatabasePlatform());
 
                 $sql .= $col . ' AS ' . $columnAlias;
 
@@ -1461,10 +1459,8 @@ public function walkSelectExpression($selectExpression)
 
                     $col = $sqlTableAlias . '.' . $quotedColumnName;
 
-                    if (isset($mapping['requireSQLConversion'])) {
-                        $type = Type::getType($mapping['type']);
-                        $col  = $type->convertToPHPValueSQL($col, $this->platform);
-                    }
+                    $type = Type::getType($mapping['type']);
+                    $col  = $type->convertToPHPValueSQL($col, $this->platform);
 
                     $sqlParts[] = $col . ' AS ' . $columnAlias;
 
@@ -1492,10 +1488,8 @@ public function walkSelectExpression($selectExpression)
 
                             $col = $sqlTableAlias . '.' . $quotedColumnName;
 
-                            if (isset($mapping['requireSQLConversion'])) {
-                                $type = Type::getType($mapping['type']);
-                                $col  = $type->convertToPHPValueSQL($col, $this->platform);
-                            }
+                            $type = Type::getType($mapping['type']);
+                            $col  = $type->convertToPHPValueSQL($col, $this->platform);
 
                             $sqlParts[] = $col . ' AS ' . $columnAlias;
 
@@ -1611,10 +1605,8 @@ public function walkNewObject($newObjectExpression, $newObjectResultAlias = null
                     $fieldType    = $fieldMapping['type'];
                     $col          = trim($e->dispatch($this));
 
-                    if (isset($fieldMapping['requireSQLConversion'])) {
-                        $type = Type::getType($fieldType);
-                        $col  = $type->convertToPHPValueSQL($col, $this->platform);
-                    }
+                    $type = Type::getType($fieldType);
+                    $col  = $type->convertToPHPValueSQL($col, $this->platform);
 
                     $sqlSelectExpressions[] = $col . ' AS ' . $columnAlias;
                     break;

diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -383,9 +383,6 @@
     </RedundantConditionGivenDocblockType>
   </file>
   <file src="lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php">
-    <DeprecatedMethod occurrences="1">
-      <code>canRequireSQLConversion</code>
-    </DeprecatedMethod>
     <DeprecatedProperty occurrences="4">
       <code>$this-&gt;columnNames</code>
       <code>$this-&gt;columnNames</code>

diff --git a/tests/Doctrine/Tests/DbalTypes/NegativeToPositiveType.php b/tests/Doctrine/Tests/DbalTypes/NegativeToPositiveType.php
@@ -24,14 +24,6 @@ public function getSQLDeclaration(array $column, AbstractPlatform $platform): st
         return $platform->getIntegerTypeDeclarationSQL($column);
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function canRequireSQLConversion()
-    {
-        return true;
-    }
-
     /**
      * {@inheritdoc}
      */

diff --git a/tests/Doctrine/Tests/DbalTypes/UpperCaseStringType.php b/tests/Doctrine/Tests/DbalTypes/UpperCaseStringType.php
@@ -16,14 +16,6 @@ public function getName(): string
         return self::NAME;
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function canRequireSQLConversion()
-    {
-        return true;
-    }
-
     /**
      * {@inheritdoc}
      */

diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2012Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2012Test.php
@@ -182,14 +182,6 @@ public function convertToDatabaseValueSQL($sqlExpr, AbstractPlatform $platform):
         return sprintf('UPPER(%s)', $sqlExpr);
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function canRequireSQLConversion()
-    {
-        return true;
-    }
-
     public function getName(): string
     {
         return self::MYTYPE;

diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2224Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2224Test.php
@@ -63,14 +63,6 @@ public function getName(): string
         return 'DDC2224Type';
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function canRequireSQLConversion()
-    {
-        return true;
-    }
-
     /**
      * {@inheritdoc}
      */

diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC5684Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC5684Test.php
@@ -81,11 +81,6 @@ public function getName(): string
     {
         return self::class;
     }
-
-    public function requiresSQLCommentHint(AbstractPlatform $platform): bool
-    {
-        return true;
-    }
 }
 
 class DDC5684ObjectId

diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH8061Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH8061Test.php
@@ -66,11 +66,6 @@ public function getName(): string
         return 'GH8061';
     }
 
-    public function canRequireSQLConversion(): bool
-    {
-        return true;
-    }
-
     public function convertToPHPValueSQL($sqlExpr, $platform): string
     {
         return sprintf('DatabaseFunction(%s)', $sqlExpr);

diff --git a/tests/Doctrine/Tests/ORM/Functional/TypeValueSqlTest.php b/tests/Doctrine/Tests/ORM/Functional/TypeValueSqlTest.php
@@ -44,7 +44,7 @@ public function testUpperCaseStringType(): void
 
         $this->_em->clear();
 
-        $entity = $this->_em->find('\Doctrine\Tests\Models\CustomType\CustomTypeUpperCase', $id);
+        $entity = $this->_em->find(CustomTypeUpperCase::class, $id);
 
         self::assertEquals('foo', $entity->lowerCaseString, 'Entity holds lowercase string');
         self::assertEquals('FOO', $this->_em->getConnection()->fetchOne('select lowerCaseString from customtype_uppercases where id=' . $entity->id . ''), 'Database holds uppercase string');
@@ -66,7 +66,7 @@ public function testUpperCaseStringTypeWhenColumnNameIsDefined(): void
 
         $this->_em->clear();
 
-        $entity = $this->_em->find('\Doctrine\Tests\Models\CustomType\CustomTypeUpperCase', $id);
+        $entity = $this->_em->find(CustomTypeUpperCase::class, $id);
         self::assertEquals('foo', $entity->namedLowerCaseString, 'Entity holds lowercase string');
         self::assertEquals('FOO', $this->_em->getConnection()->fetchOne('select named_lower_case_string from customtype_uppercases where id=' . $entity->id . ''), 'Database holds uppercase string');
 
@@ -79,7 +79,7 @@ public function testUpperCaseStringTypeWhenColumnNameIsDefined(): void
 
         $this->_em->clear();
 
-        $entity = $this->_em->find('\Doctrine\Tests\Models\CustomType\CustomTypeUpperCase', $id);
+        $entity = $this->_em->find(CustomTypeUpperCase::class, $id);
         self::assertEquals('bar', $entity->namedLowerCaseString, 'Entity holds lowercase string');
         self::assertEquals('BAR', $this->_em->getConnection()->fetchOne('select named_lower_case_string from customtype_uppercases where id=' . $entity->id . ''), 'Database holds uppercase string');
     }