From fa844b12744dc924ffc25938c5e75cb5d73ebb70 Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Sat, 30 Apr 2022 11:23:52 -0700 Subject: [PATCH] Remove support for Type::canRequireSQLConversion() --- UPGRADE.md | 8 +++++++ ...-conversion-using-custom-mapping-types.rst | 5 ---- .../ORM/Mapping/ClassMetadataInfo.php | 10 -------- lib/Doctrine/ORM/Mapping/MappingException.php | 18 -------------- .../AbstractEntityInheritancePersister.php | 6 ++--- .../Entity/BasicEntityPersister.php | 12 ++++------ .../ORM/Query/ResultSetMappingBuilder.php | 6 ++--- lib/Doctrine/ORM/Query/SqlWalker.php | 24 +++++++------------ psalm-baseline.xml | 3 --- .../DbalTypes/NegativeToPositiveType.php | 8 ------- .../Tests/DbalTypes/UpperCaseStringType.php | 8 ------- .../ORM/Functional/Ticket/DDC2012Test.php | 8 ------- .../ORM/Functional/Ticket/DDC2224Test.php | 8 ------- .../ORM/Functional/Ticket/DDC5684Test.php | 5 ---- .../ORM/Functional/Ticket/GH8061Test.php | 5 ---- .../Tests/ORM/Functional/TypeValueSqlTest.php | 6 ++--- 16 files changed, 28 insertions(+), 112 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index c0a32aaab44..2f68cdf9161 100644 --- 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 index ab9ae72c391..2a44f3725c7 100644 --- 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 index cb2e5ca11b3..a5a0baa09e0 100644 --- 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 @@ -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 index d1bea3fe443..1cae09e095a 100644 --- 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 index 4416d714dcf..397fb1f6cd6 100644 --- 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 index abbc5e09e5b..9a3de3a110c 100644 --- 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 index cce51d72559..4156e188c58 100644 --- 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 index 7539b53ec91..35062f928b7 100644 --- 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 index 6c277678859..60e44748421 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -383,9 +383,6 @@ - - canRequireSQLConversion - $this->columnNames $this->columnNames diff --git a/tests/Doctrine/Tests/DbalTypes/NegativeToPositiveType.php b/tests/Doctrine/Tests/DbalTypes/NegativeToPositiveType.php index 1fe03e4089e..a93942c6bdf 100644 --- 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 index bb7979b6957..b2224ad2139 100644 --- 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 index 3e6b609ea24..3834651a8bc 100644 --- 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 index 40d0a77c1f6..829115d2710 100644 --- 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 index 199588920da..01d81bde48c 100644 --- 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 index 08fc943e0fb..3f2d264a0ae 100644 --- 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 index a88b794ff8d..b03602fa3ef 100644 --- 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'); }