Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove support for Type::canRequireSQLConversion() #9705

Merged
merged 1 commit into from
May 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions UPGRADE.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
10 changes: 0 additions & 10 deletions lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -92,7 +91,6 @@
* originalClass?: class-string,
* originalField?: string,
* quoted?: bool,
* requireSQLConversion?: bool,
* declared?: class-string,
* declaredField?: string,
* options?: array<string, mixed>
Expand Down Expand Up @@ -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']);
Expand Down
18 changes: 0 additions & 18 deletions lib/Doctrine/ORM/Mapping/MappingException.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
12 changes: 5 additions & 7 deletions lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
}
Expand Down
6 changes: 2 additions & 4 deletions lib/Doctrine/ORM/Query/ResultSetMappingBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -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])) {
Expand Down
24 changes: 8 additions & 16 deletions lib/Doctrine/ORM/Query/SqlWalker.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down
3 changes: 0 additions & 3 deletions psalm-baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
8 changes: 0 additions & 8 deletions tests/Doctrine/Tests/DbalTypes/NegativeToPositiveType.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,6 @@ public function getSQLDeclaration(array $column, AbstractPlatform $platform): st
return $platform->getIntegerTypeDeclarationSQL($column);
}

/**
* {@inheritdoc}
*/
public function canRequireSQLConversion()
{
return true;
}

/**
* {@inheritdoc}
*/
Expand Down
8 changes: 0 additions & 8 deletions tests/Doctrine/Tests/DbalTypes/UpperCaseStringType.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,6 @@ public function getName(): string
return self::NAME;
}

/**
* {@inheritdoc}
*/
public function canRequireSQLConversion()
{
return true;
}

/**
* {@inheritdoc}
*/
Expand Down
8 changes: 0 additions & 8 deletions tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2012Test.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
8 changes: 0 additions & 8 deletions tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2224Test.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,14 +63,6 @@ public function getName(): string
return 'DDC2224Type';
}

/**
* {@inheritdoc}
*/
public function canRequireSQLConversion()
{
return true;
}

/**
* {@inheritdoc}
*/
Expand Down
5 changes: 0 additions & 5 deletions tests/Doctrine/Tests/ORM/Functional/Ticket/DDC5684Test.php
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,6 @@ public function getName(): string
{
return self::class;
}

public function requiresSQLCommentHint(AbstractPlatform $platform): bool
{
return true;
}
}

class DDC5684ObjectId
Expand Down
5 changes: 0 additions & 5 deletions tests/Doctrine/Tests/ORM/Functional/Ticket/GH8061Test.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions tests/Doctrine/Tests/ORM/Functional/TypeValueSqlTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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');

Expand All @@ -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');
}
Expand Down