From 27e8023b9b21b785b1bb4563eb14dc1001c2d711 Mon Sep 17 00:00:00 2001 From: romanb Date: Sat, 19 Dec 2009 13:38:54 +0000 Subject: [PATCH] [2.0][DDC-152] Fixed. --- .../Internal/Hydration/AbstractHydrator.php | 44 +------- lib/Doctrine/ORM/Query/ResultSetMapping.php | 106 +++++++++++++++--- lib/Doctrine/ORM/Query/SqlWalker.php | 6 +- .../ORM/Hydration/ResultSetMappingTest.php | 2 +- 4 files changed, 96 insertions(+), 62 deletions(-) diff --git a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php index 0486b4b23..8856497e5 100644 --- a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php +++ b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php @@ -189,11 +189,8 @@ abstract class AbstractHydrator $cache[$key]['fieldName'] = $this->_rsm->scalarMappings[$key]; $cache[$key]['isScalar'] = true; } else if (isset($this->_rsm->fieldMappings[$key])) { - $classMetadata = $this->_em->getClassMetadata($this->_rsm->getOwningClass($key)); $fieldName = $this->_rsm->fieldMappings[$key]; - if ( ! isset($classMetadata->reflFields[$fieldName])) { - $classMetadata = $this->_lookupDeclaringClass($classMetadata, $fieldName); - } + $classMetadata = $this->_em->getClassMetadata($this->_rsm->declaringClasses[$key]); $cache[$key]['fieldName'] = $fieldName; $cache[$key]['type'] = Type::getType($classMetadata->fieldMappings[$fieldName]['type']); $cache[$key]['isIdentifier'] = $classMetadata->isIdentifier($fieldName); @@ -254,11 +251,8 @@ abstract class AbstractHydrator $cache[$key]['fieldName'] = $this->_rsm->scalarMappings[$key]; $cache[$key]['isScalar'] = true; } else if (isset($this->_rsm->fieldMappings[$key])) { - $classMetadata = $this->_em->getClassMetadata($this->_rsm->getOwningClass($key)); $fieldName = $this->_rsm->fieldMappings[$key]; - if ( ! isset($classMetadata->reflFields[$fieldName])) { - $classMetadata = $this->_lookupDeclaringClass($classMetadata, $fieldName); - } + $classMetadata = $this->_em->getClassMetadata($this->_rsm->declaringClasses[$key]); $cache[$key]['fieldName'] = $fieldName; $cache[$key]['type'] = Type::getType($classMetadata->getTypeOfField($fieldName)); $cache[$key]['dqlAlias'] = $this->_rsm->columnOwnerMap[$key]; @@ -284,38 +278,4 @@ abstract class AbstractHydrator return $rowData; } - - /** - * Looks up the declaring class of a field in a class hierarchy. - * - * We want to make the conversion to field names while iterating over the result - * set for best performance. By doing this at that point, we can avoid re-iterating over - * the data just to convert the column names to field names. - * - * However, when this is happening, we don't know the real - * class name to instantiate yet (the row data may target a sub-type), hence - * this method looks up the field name in the subclass mappings if it's not - * found on this class mapping. - * This lookup on subclasses is costly but happens only *once* for a column - * during hydration because the hydrator caches effectively. - * - * @return string The field name. - * @throws HydrationException If the field name could not be found. - * @todo Remove. See inline FIXME comment. - */ - private function _lookupDeclaringClass($class, $fieldName) - { - // FIXME: What if two subclasses declare a (mapped) field with the same name? - // We probably need to encode the information to which subclass a field - // belongs in the column alias / result set mapping. - // This would solve the issue and would probably make this lookup superfluous. - foreach ($class->subClasses as $subClass) { - $subClassMetadata = $this->_em->getClassMetadata($subClass); - if ($subClassMetadata->hasField($fieldName)) { - return $subClassMetadata; - } - } - - throw new HydrationException("No owner found for field $fieldName."); - } } diff --git a/lib/Doctrine/ORM/Query/ResultSetMapping.php b/lib/Doctrine/ORM/Query/ResultSetMapping.php index 7309f38c6..3ed9705e7 100644 --- a/lib/Doctrine/ORM/Query/ResultSetMapping.php +++ b/lib/Doctrine/ORM/Query/ResultSetMapping.php @@ -33,29 +33,87 @@ namespace Doctrine\ORM\Query; * * @author Roman Borschel * @since 2.0 + * @todo Think about whether the number of lookup maps can be reduced. */ class ResultSetMapping { - /** Whether the result is mixed (contains scalar values together with field values). */ + /** + * Whether the result is mixed (contains scalar values together with field values). + * + * @ignore + * @var boolean + */ public $isMixed = false; - /** Maps alias names to ClassMetadata descriptors. */ + /** + * Maps alias names to class names. + * + * @ignore + * @var array + */ public $aliasMap = array(); - /** Maps alias names to related association field names. */ + /** + * Maps alias names to related association field names. + * + * @ignore + * @var array + */ public $relationMap = array(); - /** Maps alias names to parent alias names. */ + /** + * Maps alias names to parent alias names. + * + * @ignore + * @var array + */ public $parentAliasMap = array(); - /** Maps column names in the result set to field names for each class. */ + /** + * Maps column names in the result set to field names for each class. + * + * @ignore + * @var array + */ public $fieldMappings = array(); - /** Maps column names in the result set to the alias to use in the mapped result. */ + /** + * Maps column names in the result set to the alias/field name to use in the mapped result. + * + * @ignore + * @var array + */ public $scalarMappings = array(); - /** Maps column names of meta columns (foreign keys, discriminator columns, ...) to field names. */ + /** + * Maps column names of meta columns (foreign keys, discriminator columns, ...) to field names. + * + * @ignore + * @var array + */ public $metaMappings = array(); - /** Maps column names in the result set to the alias they belong to. */ + /** + * Maps column names in the result set to the alias they belong to. + * + * @ignore + * @var array + */ public $columnOwnerMap = array(); - /** List of columns in the result set that are used as discriminator columns. */ + /** + * List of columns in the result set that are used as discriminator columns. + * + * @ignore + * @var array + */ public $discriminatorColumns = array(); - /** Maps alias names to field names that should be used for indexing. */ + /** + * Maps alias names to field names that should be used for indexing. + * + * @ignore + * @var array + */ public $indexByMap = array(); + /** + * Map from column names to class names that declare the field the column is mapped to. + * + * @ignore + * @var array + */ + public $declaringClasses = array(); /** * Adds an entity result to this ResultSetMapping. @@ -63,6 +121,7 @@ class ResultSetMapping * @param string $class The class name of the entity. * @param string $alias The alias for the class. The alias must be unique among all entity * results or joined entity results within this ResultSetMapping. + * @todo Rename: addRootEntity */ public function addEntityResult($class, $alias) { @@ -77,6 +136,7 @@ class ResultSetMapping * @param string $alias The alias of the entity result or joined entity result the discriminator * column should be used for. * @param string $discrColumn The name of the discriminator column in the SQL result set. + * @todo Rename: addDiscriminatorColumn */ public function setDiscriminatorColumn($alias, $discrColumn) { @@ -113,6 +173,7 @@ class ResultSetMapping * * @param string $columnName The name of the column in the SQL result set. * @return boolean + * @todo Rename: isField */ public function isFieldResult($columnName) { @@ -120,16 +181,26 @@ class ResultSetMapping } /** - * Adds a field result that is part of an entity result or joined entity result. + * Adds a field to the result that belongs to an entity or joined entity. * - * @param string $alias The alias of the entity result or joined entity result. + * @param string $alias The alias of the root entity or joined entity to which the field belongs. * @param string $columnName The name of the column in the SQL result set. - * @param string $fieldName The name of the field on the (joined) entity. + * @param string $fieldName The name of the field on the declaring class. + * @param string $declaringClass The name of the class that declares/owns the specified field. + * When $alias refers to a superclass in a mapped hierarchy but + * the field $fieldName is defined on a subclass, specify that here. + * If not specified, the field is assumed to belong to the class + * designated by $alias. + * @todo Rename: addField */ - public function addFieldResult($alias, $columnName, $fieldName) + public function addFieldResult($alias, $columnName, $fieldName, $declaringClass = null) { + // column name (in result set) => field name $this->fieldMappings[$columnName] = $fieldName; + // column name => alias of owner $this->columnOwnerMap[$columnName] = $alias; + // field name => class name of declaring class + $this->declaringClasses[$columnName] = $declaringClass ?: $this->aliasMap[$alias]; if ( ! $this->isMixed && $this->scalarMappings) { $this->isMixed = true; } @@ -142,6 +213,7 @@ class ResultSetMapping * @param string $alias The unique alias to use for the joined entity. * @param string $parentAlias The alias of the entity result that is the parent of this joined result. * @param object $relation The association field that connects the parent entity result with the joined entity result. + * @todo Rename: addJoinedEntity */ public function addJoinedEntityResult($class, $alias, $parentAlias, $relation) { @@ -155,6 +227,7 @@ class ResultSetMapping * * @param string $columnName The name of the column in the SQL result set. * @param string $alias The result alias with which the scalar result should be placed in the result structure. + * @todo Rename: addScalar */ public function addScalarResult($columnName, $alias) { @@ -169,6 +242,7 @@ class ResultSetMapping * * @param string $columName The name of the column in the SQL result set. * @return boolean + * @todo Rename: isScalar */ public function isScalarResult($columnName) { @@ -204,9 +278,9 @@ class ResultSetMapping * @param string $columnName * @return string */ - public function getOwningClass($columnName) + public function getDeclaringClass($columnName) { - return $this->aliasMap[$this->columnOwnerMap[$columnName]]; + return $this->declaringClasses[$columnName]; } /** diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index 7a28fc6c2..418efd875 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -763,7 +763,7 @@ class SqlWalker implements TreeWalker $sql .= $sqlTableAlias . '.' . $columnName . ' AS ' . $columnAlias; $columnAlias = $this->_platform->getSqlResultCasing($columnAlias); - $this->_rsm->addFieldResult($dqlAlias, $columnAlias, $fieldName); + $this->_rsm->addFieldResult($dqlAlias, $columnAlias, $fieldName, $class->name); } else { throw DoctrineException::invalidPathExpression($expr->type); } @@ -822,7 +822,7 @@ class SqlWalker implements TreeWalker . ' AS ' . $columnAlias; $columnAlias = $this->_platform->getSqlResultCasing($columnAlias); - $this->_rsm->addFieldResult($dqlAlias, $columnAlias, $fieldName); + $this->_rsm->addFieldResult($dqlAlias, $columnAlias, $fieldName, $class->name); } // Add any additional fields of subclasses (excluding inherited fields) @@ -845,7 +845,7 @@ class SqlWalker implements TreeWalker . ' AS ' . $columnAlias; $columnAlias = $this->_platform->getSqlResultCasing($columnAlias); - $this->_rsm->addFieldResult($dqlAlias, $columnAlias, $fieldName); + $this->_rsm->addFieldResult($dqlAlias, $columnAlias, $fieldName, $subClassName); } // Add join columns (foreign keys) of the subclass diff --git a/tests/Doctrine/Tests/ORM/Hydration/ResultSetMappingTest.php b/tests/Doctrine/Tests/ORM/Hydration/ResultSetMappingTest.php index 8155dbd51..562fd4d87 100644 --- a/tests/Doctrine/Tests/ORM/Hydration/ResultSetMappingTest.php +++ b/tests/Doctrine/Tests/ORM/Hydration/ResultSetMappingTest.php @@ -43,7 +43,7 @@ class ResultSetMappingTest extends \Doctrine\Tests\OrmTestCase $this->assertFalse($this->_rsm->isScalarResult('name')); $this->assertTrue($this->_rsm->getClassName('u') == 'Doctrine\Tests\Models\CMS\CmsUser'); - $class = $this->_rsm->getOwningClass('id'); + $class = $this->_rsm->getDeclaringClass('id'); $this->assertTrue($class == 'Doctrine\Tests\Models\CMS\CmsUser'); $this->assertEquals('u', $this->_rsm->getEntityAlias('id'));