From d937d1fc82aa4d2a28e9a9baa5efa79e4516b33c Mon Sep 17 00:00:00 2001 From: Stefan Kleff Date: Tue, 12 Mar 2013 16:17:14 +0100 Subject: [PATCH 1/3] Fixed typo in hints. Caused slow loading of eager entities. --- lib/Doctrine/ORM/Persisters/BasicEntityPersister.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php index c6bd8e847..570f27ea8 100644 --- a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php @@ -851,7 +851,7 @@ class BasicEntityPersister $stmt = $this->conn->executeQuery($query, $params, $types); $hydrator = $this->em->newHydrator(($this->selectJoinSql) ? Query::HYDRATE_OBJECT : Query::HYDRATE_SIMPLEOBJECT); - return $hydrator->hydrateAll($stmt, $this->rsm, array('deferEagerLoads' => true)); + return $hydrator->hydrateAll($stmt, $this->rsm, array('deferEagerLoad' => true)); } /** @@ -908,7 +908,7 @@ class BasicEntityPersister $hydrator = $this->em->newHydrator(($this->selectJoinSql) ? Query::HYDRATE_OBJECT : Query::HYDRATE_SIMPLEOBJECT); - return $hydrator->hydrateAll($stmt, $this->rsm, array('deferEagerLoads' => true)); + return $hydrator->hydrateAll($stmt, $this->rsm, array('deferEagerLoad' => true)); } /** @@ -939,7 +939,7 @@ class BasicEntityPersister private function loadArrayFromStatement($assoc, $stmt) { $rsm = $this->rsm; - $hints = array('deferEagerLoads' => true); + $hints = array('deferEagerLoad' => true); if (isset($assoc['indexBy'])) { $rsm = clone ($this->rsm); // this is necessary because the "default rsm" should be changed. @@ -962,8 +962,8 @@ class BasicEntityPersister { $rsm = $this->rsm; $hints = array( - 'deferEagerLoads' => true, - 'collection' => $coll + 'deferEagerLoad' => true, + 'collection' => $coll ); if (isset($assoc['indexBy'])) { From 057e86eb275eb0412be9e5216fe01e7b3542c6bf Mon Sep 17 00:00:00 2001 From: Stefan Kleff Date: Wed, 13 Mar 2013 12:29:17 +0100 Subject: [PATCH 2/3] Added test based on https://github.com/Ocramius/doctrine2/commit/e468ced00bd107f40fd08beca4c7cc5a2492a418 --- .../ORM/Functional/Ticket/DDC2346Test.php | 109 ++++++++++++++++++ 1 file changed, 109 insertions(+) create mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2346Test.php diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2346Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2346Test.php new file mode 100644 index 000000000..8bc803f29 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2346Test.php @@ -0,0 +1,109 @@ +_schemaTool->createSchema(array( + $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC2346Foo'), + $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC2346Bar'), + $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC2346Baz'), + )); + + $this->logger = new DebugStack(); + } + + /** + * Verifies that fetching a OneToMany association with fetch="EAGER" does not cause N+1 queries + */ + public function testIssue() + { + $foo1 = new DDC2346Foo(); + $foo2 = new DDC2346Foo(); + + $baz1 = new DDC2346Baz(); + $baz2 = new DDC2346Baz(); + + $baz1->foo = $foo1; + $baz2->foo = $foo2; + + $foo1->bars[] = $baz1; + $foo1->bars[] = $baz2; + + $this->_em->persist($foo1); + $this->_em->persist($foo2); + $this->_em->persist($baz1); + $this->_em->persist($baz2); + + $this->_em->flush(); + $this->_em->clear(); + + $this->_em->getConnection()->getConfiguration()->setSQLLogger($this->logger); + + $fetchedBazs = $this->_em->getRepository(__NAMESPACE__ . '\\DDC2346Baz')->findAll(); + + $this->assertCount(2, $fetchedBazs); + $this->assertCount(2, $this->logger->queries, 'The total number of executed queries is 2, and not n+1'); + } +} + +/** @Entity */ +class DDC2346Foo +{ + /** @Id @Column(type="integer") @GeneratedValue */ + public $id; + + /** + * @var DDC2346Bar[]|\Doctrine\Common\Collections\Collection + * + * @OneToMany(targetEntity="DDC2346Bar", mappedBy="foo") + */ + public $bars; + + /** Constructor */ + public function __construct() { + $this->bars = new ArrayCollection(); + } +} + +/** + * @Entity + * @InheritanceType("JOINED") + * @DiscriminatorColumn(name="discr", type="string") + * @DiscriminatorMap({"baz" = "DDC2346Baz"}) + */ +class DDC2346Bar +{ + /** @Id @Column(type="integer") @GeneratedValue */ + public $id; + + /** @ManyToOne(targetEntity="DDC2346Foo", inversedBy="bars", fetch="EAGER") */ + public $foo; +} + + +/** + * @Entity + */ +class DDC2346Baz extends DDC2346Bar +{ + +} \ No newline at end of file From e561f47cb2205565eb873f0643637477bfcfc2ff Mon Sep 17 00:00:00 2001 From: Stefan Kleff Date: Mon, 8 Apr 2013 11:27:22 +0200 Subject: [PATCH 3/3] Added constant --- .../ORM/Internal/Hydration/ObjectHydrator.php | 7 ++++--- lib/Doctrine/ORM/Persisters/BasicEntityPersister.php | 10 +++++----- lib/Doctrine/ORM/UnitOfWork.php | 11 +++++++++-- 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php b/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php index 04d741cbb..fafc497c0 100644 --- a/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php +++ b/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php @@ -19,6 +19,7 @@ namespace Doctrine\ORM\Internal\Hydration; +use Doctrine\ORM\UnitOfWork; use PDO; use Doctrine\ORM\Mapping\ClassMetadata; use Doctrine\ORM\PersistentCollection; @@ -94,8 +95,8 @@ class ObjectHydrator extends AbstractHydrator $this->resultCounter = 0; - if ( ! isset($this->_hints['deferEagerLoad'])) { - $this->_hints['deferEagerLoad'] = true; + if ( ! isset($this->_hints[UnitOfWork::HINT_DEFEREAGERLOAD])) { + $this->_hints[UnitOfWork::HINT_DEFEREAGERLOAD] = true; } foreach ($this->_rsm->aliasMap as $dqlAlias => $className) { @@ -152,7 +153,7 @@ class ObjectHydrator extends AbstractHydrator */ protected function cleanup() { - $eagerLoad = (isset($this->_hints['deferEagerLoad'])) && $this->_hints['deferEagerLoad'] == true; + $eagerLoad = (isset($this->_hints[UnitOfWork::HINT_DEFEREAGERLOAD])) && $this->_hints[UnitOfWork::HINT_DEFEREAGERLOAD] == true; parent::cleanup(); diff --git a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php index 570f27ea8..894ac3ffe 100644 --- a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php @@ -851,7 +851,7 @@ class BasicEntityPersister $stmt = $this->conn->executeQuery($query, $params, $types); $hydrator = $this->em->newHydrator(($this->selectJoinSql) ? Query::HYDRATE_OBJECT : Query::HYDRATE_SIMPLEOBJECT); - return $hydrator->hydrateAll($stmt, $this->rsm, array('deferEagerLoad' => true)); + return $hydrator->hydrateAll($stmt, $this->rsm, array(UnitOfWork::HINT_DEFEREAGERLOAD => true)); } /** @@ -908,7 +908,7 @@ class BasicEntityPersister $hydrator = $this->em->newHydrator(($this->selectJoinSql) ? Query::HYDRATE_OBJECT : Query::HYDRATE_SIMPLEOBJECT); - return $hydrator->hydrateAll($stmt, $this->rsm, array('deferEagerLoad' => true)); + return $hydrator->hydrateAll($stmt, $this->rsm, array(UnitOfWork::HINT_DEFEREAGERLOAD => true)); } /** @@ -939,7 +939,7 @@ class BasicEntityPersister private function loadArrayFromStatement($assoc, $stmt) { $rsm = $this->rsm; - $hints = array('deferEagerLoad' => true); + $hints = array(UnitOfWork::HINT_DEFEREAGERLOAD => true); if (isset($assoc['indexBy'])) { $rsm = clone ($this->rsm); // this is necessary because the "default rsm" should be changed. @@ -962,8 +962,8 @@ class BasicEntityPersister { $rsm = $this->rsm; $hints = array( - 'deferEagerLoad' => true, - 'collection' => $coll + UnitOfWork::HINT_DEFEREAGERLOAD => true, + 'collection' => $coll ); if (isset($assoc['indexBy'])) { diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 4c3f3720c..fa569caf7 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -76,6 +76,13 @@ class UnitOfWork implements PropertyChangedListener */ const STATE_REMOVED = 4; + /** + * Hint used to collect all primary keys of associated entities during hydration + * and execute it in a dedicated query afterwards + * @see https://doctrine-orm.readthedocs.org/en/latest/reference/dql-doctrine-query-language.html?highlight=eager#temporarily-change-fetch-mode-in-dql + */ + const HINT_DEFEREAGERLOAD = 'deferEagerLoad'; + /** * The identity map that holds references to all managed entities that have * an identity. The entities are grouped by their class name. @@ -2616,7 +2623,7 @@ class UnitOfWork implements PropertyChangedListener // this association is marked as eager fetch, and its an uninitialized proxy (wtf!) // then we can append this entity for eager loading! if ($hints['fetchMode'][$class->name][$field] == ClassMetadata::FETCH_EAGER && - isset($hints['deferEagerLoad']) && + isset($hints[self::HINT_DEFEREAGERLOAD]) && !$targetClass->isIdentifierComposite && $newValue instanceof Proxy && $newValue->__isInitialized__ === false) { @@ -2641,7 +2648,7 @@ class UnitOfWork implements PropertyChangedListener break; // Deferred eager load only works for single identifier classes - case (isset($hints['deferEagerLoad']) && ! $targetClass->isIdentifierComposite): + case (isset($hints[self::HINT_DEFEREAGERLOAD]) && ! $targetClass->isIdentifierComposite): // TODO: Is there a faster approach? $this->eagerLoadingEntities[$targetClass->rootEntityName][$relatedIdHash] = current($associatedId);