From 077ae9cee92cd4f2d084bc315e76e3376ff1904b Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Wed, 9 Mar 2011 23:14:54 +0100 Subject: [PATCH] [DDC-914] Fetch join many-to-one/one-to-one associations configured as FETCH_EAGER inside the persisters. --- .../AbstractEntityInheritancePersister.php | 6 +- .../ORM/Persisters/BasicEntityPersister.php | 98 ++++++++++++++++--- .../ORM/Persisters/SingleTablePersister.php | 4 +- lib/Doctrine/ORM/UnitOfWork.php | 1 + .../ORM/Functional/Ticket/DDC1050Test.php | 37 +++++++ .../ORM/Functional/Ticket/DDC633Test.php | 7 +- 6 files changed, 130 insertions(+), 23 deletions(-) create mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1050Test.php diff --git a/lib/Doctrine/ORM/Persisters/AbstractEntityInheritancePersister.php b/lib/Doctrine/ORM/Persisters/AbstractEntityInheritancePersister.php index e74bf5eb1..da50b3be8 100644 --- a/lib/Doctrine/ORM/Persisters/AbstractEntityInheritancePersister.php +++ b/lib/Doctrine/ORM/Persisters/AbstractEntityInheritancePersister.php @@ -103,16 +103,16 @@ abstract class AbstractEntityInheritancePersister extends BasicEntityPersister /** * {@inheritdoc} */ - protected function _getSelectColumnSQL($field, ClassMetadata $class) + protected function _getSelectColumnSQL($field, ClassMetadata $class, $alias = 'r') { $columnName = $class->columnNames[$field]; - $sql = $this->_getSQLTableAlias($class->name) . '.' . $class->getQuotedColumnName($field, $this->_platform); + $sql = $this->_getSQLTableAlias($class->name, $alias == 'r' ? '' : $alias) . '.' . $class->getQuotedColumnName($field, $this->_platform); $columnAlias = $this->_platform->getSQLResultCasing($columnName . $this->_sqlAliasCounter++); if ( ! isset($this->_resultColumnNames[$columnAlias])) { $this->_resultColumnNames[$columnAlias] = $columnName; $this->declaringClassMap[$columnAlias] = $class; } - $this->_rsm->addFieldResult('r', $columnAlias, $field, $class->name); + $this->_rsm->addFieldResult($alias, $columnAlias, $field, $class->name); return "$sql AS $columnAlias"; } diff --git a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php index 4174e3dae..d956fe240 100644 --- a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php @@ -152,6 +152,14 @@ class BasicEntityPersister * @var string */ protected $_selectColumnListSql; + + /** + * The JOIN SQL fragement used to eagerly load all many-to-one and one-to-one + * associations configured as FETCH_EAGER, aswell as all inverse one-to-one associations. + * + * @var string + */ + protected $_selectJoinSql; /** * Counter for creating unique SQL table and column aliases. @@ -915,7 +923,7 @@ class BasicEntityPersister return $this->_platform->modifyLimitQuery('SELECT ' . $this->_getSelectColumnListSQL() . $this->_platform->appendLockHint(' FROM ' . $this->_class->getQuotedTableName($this->_platform) . ' ' . $this->_getSQLTableAlias($this->_class->name), $lockMode) - . $joinSql + . $this->_selectJoinSql . $joinSql . ($conditionSql ? ' WHERE ' . $conditionSql : '') . $orderBySql, $limit, $offset) . $lockSql; @@ -977,18 +985,54 @@ class BasicEntityPersister $columnList .= $this->_getSelectColumnSQL($field, $this->_class); } - foreach ($this->_class->associationMappings as $assoc) { - if ($assoc['isOwningSide'] && $assoc['type'] & ClassMetadata::TO_ONE) { - foreach ($assoc['targetToSourceKeyColumns'] as $srcColumn) { + $this->_selectJoinSql = ''; + $eagerAliasCounter = 0; + foreach ($this->_class->associationMappings as $assocField => $assoc) { + $assocColumnSQL = $this->_getSelectColumnAssociationSQL($assocField, $assoc, $this->_class); + if ($assocColumnSQL) { + if ($columnList) $columnList .= ', '; + $columnList .= $assocColumnSQL; + } + + if ($assoc['fetch'] == ClassMetadata::FETCH_EAGER && $assoc['type'] & ClassMetadata::TO_ONE) { + $eagerEntity = $this->_em->getClassMetadata($assoc['targetEntity']); + if ($eagerEntity->inheritanceType != ClassMetadata::INHERITANCE_TYPE_NONE) { + continue; // now this is why you shouldn't use inheritance + } + + $assocAlias = 'e' + ($eagerAliasCounter++); + $this->_rsm->addJoinedEntityResult($assoc['targetEntity'], $assocAlias, 'r', $assocField); + + foreach ($eagerEntity->fieldNames AS $field) { if ($columnList) $columnList .= ', '; - - $columnAlias = $srcColumn . $this->_sqlAliasCounter++; - $columnList .= $this->_getSQLTableAlias($this->_class->name) . ".$srcColumn AS $columnAlias"; - $resultColumnName = $this->_platform->getSQLResultCasing($columnAlias); - if ( ! isset($this->_resultColumnNames[$resultColumnName])) { - $this->_resultColumnNames[$resultColumnName] = $srcColumn; + $columnList .= $this->_getSelectColumnSQL($field, $eagerEntity, $assocAlias); + } + + foreach ($eagerEntity->associationMappings as $assoc2Field => $assoc2) { + $assoc2ColumnSQL = $this->_getSelectColumnAssociationSQL($assoc2Field, $assoc2, $eagerEntity); + if ($assoc2ColumnSQL) { + if ($columnList) $columnList .= ', '; + $columnList .= $assoc2ColumnSQL; + } + } + $this->_selectJoinSql .= ' LEFT JOIN'; + if ($assoc['isOwningSide']) { + $this->_selectJoinSql .= ' ' . $eagerEntity->table['name'] . ' ' . $this->_getSQLTableAlias($eagerEntity->name, $assocAlias) .' ON '; + + foreach ($assoc['sourceToTargetKeyColumns'] AS $sourceCol => $targetCol) { + $this->_selectJoinSql .= $this->_getSQLTableAlias($assoc['sourceEntity']) . '.'.$sourceCol.' = ' . + $this->_getSQLTableAlias($assoc['targetEntity'], $assocAlias) . '.'.$targetCol.' '; + } + } else { + $eagerEntity = $this->_em->getClassMetadata($assoc['sourceEntity']); + $owningAssoc = $eagerEntity->getAssociationMapping($assoc['mappedBy']); + + $this->_selectJoinSql .= ' ' . $eagerEntity->table['name'] . ' ' . $this->_getSQLTableAlias($eagerEntity->name, $assocAlias) .' ON '; + + foreach ($owningAssoc['sourceToTargetKeyColumns'] AS $sourceCol => $targetCol) { + $this->_selectJoinSql .= $this->_getSQLTableAlias($owningAssoc['sourceEntity'], $assocAlias) . '.'.$sourceCol.' = ' . + $this->_getSQLTableAlias($assoc['targetEntity']) . '.' . $targetCol . ' '; } - $this->_rsm->addMetaResult('r', $this->_platform->getSQLResultCasing($columnAlias), $srcColumn); } } } @@ -997,6 +1041,25 @@ class BasicEntityPersister return $this->_selectColumnListSql; } + + protected function _getSelectColumnAssociationSQL($field, $assoc, ClassMetadata $class) + { + $columnList = ''; + if ($assoc['isOwningSide'] && $assoc['type'] & ClassMetadata::TO_ONE) { + foreach ($assoc['targetToSourceKeyColumns'] as $srcColumn) { + if ($columnList) $columnList .= ', '; + + $columnAlias = $srcColumn . $this->_sqlAliasCounter++; + $columnList .= $this->_getSQLTableAlias($class->name) . ".$srcColumn AS $columnAlias"; + $resultColumnName = $this->_platform->getSQLResultCasing($columnAlias); + if ( ! isset($this->_resultColumnNames[$resultColumnName])) { + $this->_resultColumnNames[$resultColumnName] = $srcColumn; + } + $this->_rsm->addMetaResult('r', $this->_platform->getSQLResultCasing($columnAlias), $srcColumn); + } + } + return $columnList; + } /** * Gets the SQL join fragment used when selecting entities from a @@ -1100,16 +1163,17 @@ class BasicEntityPersister * @param string $field The field name. * @param ClassMetadata $class The class that declares this field. The table this class is * mapped to must own the column for the given field. + * @param string $alias */ - protected function _getSelectColumnSQL($field, ClassMetadata $class) + protected function _getSelectColumnSQL($field, ClassMetadata $class, $alias = 'r') { $columnName = $class->columnNames[$field]; - $sql = $this->_getSQLTableAlias($class->name) . '.' . $class->getQuotedColumnName($field, $this->_platform); + $sql = $this->_getSQLTableAlias($class->name, $alias == 'r' ? '' : $alias) . '.' . $class->getQuotedColumnName($field, $this->_platform); $columnAlias = $this->_platform->getSQLResultCasing($columnName . $this->_sqlAliasCounter++); if ( ! isset($this->_resultColumnNames[$columnAlias])) { $this->_resultColumnNames[$columnAlias] = $columnName; } - $this->_rsm->addFieldResult('r', $columnAlias, $field); + $this->_rsm->addFieldResult($alias, $columnAlias, $field); return "$sql AS $columnAlias"; } @@ -1121,8 +1185,12 @@ class BasicEntityPersister * @return string The SQL table alias. * @todo Reconsider. Binding table aliases to class names is not such a good idea. */ - protected function _getSQLTableAlias($className) + protected function _getSQLTableAlias($className, $assocName = '') { + if ($assocName) { + $className .= '#'.$assocName; + } + if (isset($this->_sqlTableAliases[$className])) { return $this->_sqlTableAliases[$className]; } diff --git a/lib/Doctrine/ORM/Persisters/SingleTablePersister.php b/lib/Doctrine/ORM/Persisters/SingleTablePersister.php index c0d8c2bfa..94450fd89 100644 --- a/lib/Doctrine/ORM/Persisters/SingleTablePersister.php +++ b/lib/Doctrine/ORM/Persisters/SingleTablePersister.php @@ -88,9 +88,9 @@ class SingleTablePersister extends AbstractEntityInheritancePersister } /** {@inheritdoc} */ - protected function _getSQLTableAlias($className) + protected function _getSQLTableAlias($className, $assocName = '') { - return parent::_getSQLTableAlias($this->_class->rootEntityName); + return parent::_getSQLTableAlias($this->_class->rootEntityName, $assocName); } /** {@inheritdoc} */ diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 8ed9dbb54..2326ef5d1 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -1914,6 +1914,7 @@ class UnitOfWork implements PropertyChangedListener if ($assoc['type'] & ClassMetadata::TO_ONE) { if ($assoc['isOwningSide']) { $associatedId = array(); + // TODO: Is this even computed right in all cases of composite keys? foreach ($assoc['targetToSourceKeyColumns'] as $targetColumn => $srcColumn) { $joinColumnValue = isset($data[$srcColumn]) ? $data[$srcColumn] : null; if ($joinColumnValue !== null) { diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1050Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1050Test.php new file mode 100644 index 000000000..82e9590c0 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1050Test.php @@ -0,0 +1,37 @@ +markTestSkipped('performance skipped'); + $this->useModelSet('cms'); + parent::setUp(); + } + + public function testPerformance() + { + for ($i = 2; $i < 10000; ++$i) { + $user = new \Doctrine\Tests\Models\CMS\CmsUser(); + $user->status = 'developer'; + $user->username = 'jwage'+$i; + $user->name = 'Jonathan'; + $this->_em->persist($user); + } + $this->_em->flush(); + $this->_em->clear(); + + $s = microtime(true); + $users = $this->_em->getRepository('Doctrine\Tests\Models\CMS\CmsUser')->findAll(); + $e = microtime(true); + echo __FUNCTION__ . " - " . ($e - $s) . " seconds" . PHP_EOL; + } +} \ No newline at end of file diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC633Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC633Test.php index 1c7a3d171..d51bdd361 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC633Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC633Test.php @@ -24,6 +24,7 @@ class DDC633Test extends \Doctrine\Tests\OrmFunctionalTestCase /** * @group DDC-633 * @group DDC-952 + * @group DDC-914 */ public function testOneToOneEager() { @@ -39,9 +40,9 @@ class DDC633Test extends \Doctrine\Tests\OrmFunctionalTestCase $eagerAppointment = $this->_em->find(__NAMESPACE__ . '\DDC633Appointment', $app->id); - // Eager loading still produces proxies - $this->assertType('Doctrine\ORM\Proxy\Proxy', $eagerAppointment->patient); - $this->assertTrue($eagerAppointment->patient->__isInitialized__, "Proxy should already be initialized due to eager loading!"); + // Eager loading of one to one leads to fetch-join + $this->assertNotInstanceOf('Doctrine\ORM\Proxy\Proxy', $eagerAppointment->patient); + $this->assertTrue($this->_em->contains($eagerAppointment->patient)); } /**