Skip to content

Commit

Permalink
#7527 fix - resolve a parameter's value only if its type is not expli…
Browse files Browse the repository at this point in the history
…citly specified

Note: this will still lead to the `UnitOfWork#getSingleIdentifierValue()` still being
called when not specifying the type of a DQL parameter being bound via
`Doctrine\ORM\Query#setParameter()`:

```php
$query->setParameter('foo', $theValue, $theType);
```

A full parameter bind is required in order to gain back performance:

```php
$query->setParameter('foo', $theValue, $theType);
```

This is up for discussion with patch reviewers.
  • Loading branch information
Ocramius committed Dec 16, 2018
1 parent 960a437 commit 2f7508b
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 21 deletions.
2 changes: 1 addition & 1 deletion lib/Doctrine/ORM/AbstractQuery.php
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ abstract class AbstractQuery
/**
* The parameter map of this query.
*
* @var \Doctrine\Common\Collections\ArrayCollection
* @var \Doctrine\Common\Collections\ArrayCollection|\Doctrine\ORM\Query\Parameter[]
*/
protected $parameters;

Expand Down
57 changes: 39 additions & 18 deletions lib/Doctrine/ORM/Query.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,17 @@

namespace Doctrine\ORM;

use Doctrine\Common\Collections\ArrayCollection;
use Doctrine\DBAL\LockMode;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\Query\Exec\AbstractSqlExecutor;
use Doctrine\ORM\Query\Parameter;
use Doctrine\ORM\Query\ParameterTypeInferer;
use Doctrine\ORM\Query\Parser;
use Doctrine\ORM\Query\ParserResult;
use Doctrine\ORM\Query\QueryException;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\Query\ParameterTypeInferer;
use Doctrine\Common\Collections\ArrayCollection;
use Doctrine\ORM\Utility\HierarchyDiscriminatorResolver;
use function assert;

/**
* A Query object represents a DQL query.
Expand Down Expand Up @@ -387,26 +389,13 @@ private function processParameterMappings($paramMappings)
$types = [];

foreach ($this->parameters as $parameter) {
$key = $parameter->getName();
$value = $parameter->getValue();
$rsm = $this->getResultSetMapping();
$key = $parameter->getName();

if ( ! isset($paramMappings[$key])) {
throw QueryException::unknownParameter($key);
}

if (isset($rsm->metadataParameterMapping[$key]) && $value instanceof ClassMetadata) {
$value = $value->getMetadataValue($rsm->metadataParameterMapping[$key]);
}

if (isset($rsm->discriminatorParameters[$key]) && $value instanceof ClassMetadata) {
$value = array_keys(HierarchyDiscriminatorResolver::resolveDiscriminatorsForClass($value, $this->_em));
}

$value = $this->processParameterValue($value);
$type = ($parameter->getValue() === $value)
? $parameter->getType()
: ParameterTypeInferer::inferType($value);
[$value, $type] = $this->resolveParameterValue($parameter);

foreach ($paramMappings[$key] as $position) {
$types[$position] = $type;
Expand Down Expand Up @@ -439,6 +428,38 @@ private function processParameterMappings($paramMappings)
return [$sqlParams, $types];
}

/** @return mixed[] tuple of (value, type) */
private function resolveParameterValue(Parameter $parameter) : array
{
if ($parameter->typeWasSpecified()) {
return [$parameter->getValue(), $parameter->getType()];
}

$key = $parameter->getName();
$originalValue = $parameter->getValue();
$value = $originalValue;
$rsm = $this->getResultSetMapping();

assert($rsm !== null);

if ($value instanceof ClassMetadata && isset($rsm->metadataParameterMapping[$key])) {
$value = $value->getMetadataValue($rsm->metadataParameterMapping[$key]);
}

if ($value instanceof ClassMetadata && isset($rsm->discriminatorParameters[$key])) {
$value = array_keys(HierarchyDiscriminatorResolver::resolveDiscriminatorsForClass($value, $this->_em));
}

$processedValue = $this->processParameterValue($value);

return [
$processedValue,
$originalValue === $processedValue
? $parameter->getType()
: ParameterTypeInferer::inferType($processedValue),
];
}

/**
* Defines a cache driver to be used for caching queries.
*
Expand Down
15 changes: 14 additions & 1 deletion lib/Doctrine/ORM/Query/Parameter.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,13 @@ class Parameter
*/
private $type;

/**
* Whether the parameter type was explicitly specified or not
*
* @var bool
*/
private $typeSpecified;

/**
* Constructor.
*
Expand All @@ -58,7 +65,8 @@ class Parameter
*/
public function __construct($name, $value, $type = null)
{
$this->name = trim($name, ':');
$this->name = trim($name, ':');
$this->typeSpecified = $type !== null;

$this->setValue($value, $type);
}
Expand Down Expand Up @@ -104,4 +112,9 @@ public function setValue($value, $type = null)
$this->value = $value;
$this->type = $type ?: ParameterTypeInferer::inferType($value);
}

public function typeWasSpecified() : bool
{
return $this->typeSpecified;
}
}
5 changes: 4 additions & 1 deletion tests/Doctrine/Tests/ORM/QueryBuilderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -609,8 +609,11 @@ public function testSetParameter()
->setParameter('id', 1);

$parameter = new Parameter('id', 1, ParameterTypeInferer::inferType(1));
$inferred = $qb->getParameter('id');

$this->assertEquals($parameter, $qb->getParameter('id'));
self::assertSame($parameter->getValue(), $inferred->getValue());
self::assertSame($parameter->getType(), $inferred->getType());
self::assertFalse($inferred->typeWasSpecified());
}

public function testSetParameters()
Expand Down

0 comments on commit 2f7508b

Please sign in to comment.