Skip to content

Commit

Permalink
Fixed doctrine#7820 - convert identifiers for WHERE IN(?) queries b…
Browse files Browse the repository at this point in the history
…efore binding parameters

This patch introduces new internal API on the `ResultSetMapping` class, which is responsible
for finding the type of the single column identifier of a DQL query selection root.
  • Loading branch information
Ocramius committed Sep 17, 2019
1 parent 65522d9 commit 6a93622
Show file tree
Hide file tree
Showing 6 changed files with 125 additions and 5 deletions.
43 changes: 42 additions & 1 deletion lib/Doctrine/ORM/Query/ResultSetMapping.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@

namespace Doctrine\ORM\Query;

use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Mapping\MappingException;
use function array_keys;
use function array_values;
use function assert;

/**
* A ResultSetMapping describes how a result set of an SQL query maps to a Doctrine result.
*
Expand Down Expand Up @@ -583,5 +589,40 @@ public function addMetaResult($alias, $columnName, $fieldName, $isIdentifierColu

return $this;
}
}

/**
* Retrieves the DBAL type name for the single identifier column of the root of a selection.
* Composite identifiers not supported!
*
* @throws MappingException If the identifier is not a single field, or if metadata for its
* owner is incorrect/missing.
* @internal only to be used by ORM internals: do not use in downstream projects! This API is a minimal abstraction
* that only ORM internals need, and it tries to make sense of the very complex and squishy array-alike
* structure inside this class. Some assumptions are coded in here, so here be dragons.
*
*/
final public function getTypeOfSelectionRootSingleIdentifierColumn(EntityManagerInterface $em) : string
{
assert($this->isSelect);

if ($this->isIdentifierColumn !== []) {
// Identifier columns are already discovered here: we can use the first one directly.
assert($this->typeMappings !== []);

return $this->typeMappings[array_keys(array_values($this->isIdentifierColumn)[0])[0]];
}

// We are selecting entities, and the first selected entity is our root of the selection.
if ($this->aliasMap !== []) {
$metadata = $em->getClassMetadata($this->aliasMap[array_keys($this->aliasMap)[0]]);

return $metadata->getTypeOfField($metadata->getSingleIdentifierFieldName());
}

// We are selecting scalar fields - the first selected field will be assumed (!!! assumption !!!) as identifier
assert($this->scalarMappings !== []);
assert($this->typeMappings !== []);

return $this->typeMappings[array_keys($this->scalarMappings)[0]];
}
}
39 changes: 36 additions & 3 deletions lib/Doctrine/ORM/Tools/Pagination/Paginator.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,16 @@

namespace Doctrine\ORM\Tools\Pagination;

use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Mapping\MappingException;
use Doctrine\ORM\Query\Parser;
use Doctrine\ORM\QueryBuilder;
use Doctrine\ORM\Query;
use Doctrine\ORM\Query\ResultSetMapping;
use Doctrine\ORM\NoResultException;
use function array_map;
use function assert;
use function current;

/**
* The paginator can handle various complex scenarios with DQL.
Expand Down Expand Up @@ -150,14 +155,21 @@ public function getIterator()

$subQuery->setFirstResult($offset)->setMaxResults($length);

$ids = array_map('current', $subQuery->getScalarResult());
$foundIdRows = $subQuery->getScalarResult();

$whereInQuery = $this->cloneQuery($this->query);
// don't do this for an empty id array
if (count($ids) === 0) {
if ($foundIdRows === []) {
return new \ArrayIterator([]);
}

$whereInQuery = $this->cloneQuery($this->query);
$em = $subQuery->getEntityManager();
$connection = $em->getConnection();
$idType = $this->getIdentifiersQueryScalarResultType($subQuery, $em);
$ids = array_map(static function (array $row) use ($connection, $idType) {
return $connection->convertToDatabaseValue(current($row), $idType);
}, $foundIdRows);

$this->appendTreeWalker($whereInQuery, WhereInWalker::class);
$whereInQuery->setHint(WhereInWalker::HINT_PAGINATOR_ID_COUNT, count($ids));
$whereInQuery->setFirstResult(null)->setMaxResults(null);
Expand Down Expand Up @@ -282,4 +294,25 @@ private function unbindUnusedQueryParams(Query $query): void

$query->setParameters($parameters);
}

/**
* Parses a query that is supposed to fetch a set of entity identifier only,
* and retrieves the type of said identifier.
*
* @throws MappingException If metadata couldn't be loaded, or if there isn't a single
* identifier for the given query.
*/
private function getIdentifiersQueryScalarResultType(
Query $query,
EntityManagerInterface $em
) : ?string {
$rsm = (new Parser($query))
->parse()
->getResultSetMapping();

assert($rsm !== null);
assert($rsm->isSelect);

return $rsm->getTypeOfSelectionRootSingleIdentifierColumn($em);
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Doctrine\Tests\Models\ValueConversionType;

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?php

namespace Doctrine\Tests\Models\ValueConversionType;

/**
* @Entity
* @Table(name="vct_owning_manytoone_foreignkey")
*/
class OwningManyToOneIdForeignKeyEntity
{
/**
* @Id
* @ManyToOne(targetEntity=AuxiliaryEntity::class, inversedBy="associatedEntities")
* @JoinColumn(name="associated_id", referencedColumnName="id4")
*/
public $associatedEntity;
}
3 changes: 2 additions & 1 deletion tests/Doctrine/Tests/ORM/Functional/Ticket/GH7820Test.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use Doctrine\ORM\Tools\Pagination\Paginator;
use Doctrine\Tests\OrmFunctionalTestCase;
use function array_map;
use function is_string;
use function iterator_to_array;

/**
Expand Down Expand Up @@ -134,7 +135,7 @@ public function convertToPHPValue($value, AbstractPlatform $platform)
{
$text = parent::convertToPHPValue($value, $platform);

if (! \is_string($text)) {
if (! is_string($text)) {
return $text;
}

Expand Down
26 changes: 26 additions & 0 deletions tests/Doctrine/Tests/ORM/Hydration/ResultSetMappingTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,15 @@
namespace Doctrine\Tests\ORM\Hydration;

use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\Query\Parser;
use Doctrine\ORM\Query\ResultSetMapping;
use Doctrine\Tests\Models\CMS\CmsEmail;
use Doctrine\Tests\Models\CMS\CmsPhonenumber;
use Doctrine\Tests\Models\CMS\CmsUser;
use Doctrine\Tests\Models\Legacy\LegacyUser;
use Doctrine\Tests\Models\Legacy\LegacyUserReference;
use Doctrine\Tests\Models\ValueConversionType\AuxiliaryEntity;
use Doctrine\Tests\Models\ValueConversionType\OwningManyToOneIdForeignKeyEntity;

/**
* Description of ResultSetMappingTest
Expand Down Expand Up @@ -65,6 +68,7 @@ public function testBasicResultSetMapping()
$this->assertEquals('status', $this->_rsm->getFieldName('status'));
$this->assertEquals('username', $this->_rsm->getFieldName('username'));
$this->assertEquals('name', $this->_rsm->getFieldName('name'));
$this->assertSame('integer', $this->_rsm->getTypeOfSelectionRootSingleIdentifierColumn($this->_em));
}

/**
Expand Down Expand Up @@ -94,6 +98,7 @@ public function testFluentInterface()
$this->assertTrue($rms->isRelation('p'));
$this->assertTrue($rms->hasParentAlias('p'));
$this->assertTrue($rms->isMixedResult());
$this->assertSame('integer', $this->_rsm->getTypeOfSelectionRootSingleIdentifierColumn($this->_em));
}

/**
Expand Down Expand Up @@ -269,6 +274,27 @@ public function testAddNamedNativeQueryResultClass()
$this->assertEquals(CmsUser::class, $rsm->getDeclaringClass('status'));
$this->assertEquals(CmsUser::class, $rsm->getDeclaringClass('username'));
}

public function testIdentifierTypeForScalarExpression() : void
{
$rsm = (new Parser($this->_em->createQuery('SELECT e.id4 FROM ' . AuxiliaryEntity::class . ' e')))
->parse()
->getResultSetMapping();

self::assertNotNull($rsm);
self::assertSame('rot13', $rsm->getTypeOfSelectionRootSingleIdentifierColumn($this->_em));
}

public function testIdentifierTypeForRootEntityColumnThatHasAssociationAsIdentifier() : void
{
$rsm = (new Parser($this->_em->createQuery('SELECT e FROM ' . OwningManyToOneIdForeignKeyEntity::class . ' e')))
->parse()
->getResultSetMapping();

self::assertNotNull($rsm);
self::assertSame('rot13', $rsm->getTypeOfSelectionRootSingleIdentifierColumn($this->_em));
}

/**
* @group DDC-117
*/
Expand Down

0 comments on commit 6a93622

Please sign in to comment.