From 5b60f878598007dc0aa57db1c1e7219c9640b148 Mon Sep 17 00:00:00 2001 From: romanb Date: Thu, 22 Oct 2009 19:12:00 +0000 Subject: [PATCH] [2.0] Fixed some known issues with inheritance, especially class table inheritance, like join columns not being selected. --- .../ORM/Mapping/ClassMetadataFactory.php | 9 ++++- .../ORM/Mapping/ClassMetadataInfo.php | 5 +++ .../Persisters/JoinedSubclassPersister.php | 40 ++++++++++++++----- lib/Doctrine/ORM/Query/SqlWalker.php | 19 +++++++-- .../Tests/Models/Company/CompanyCar.php | 33 +++++++++++++++ .../Tests/Models/Company/CompanyManager.php | 14 +++++++ .../Functional/ClassTableInheritanceTest.php | 24 ++++++++++- .../ORM/Query/SelectSqlGenerationTest.php | 2 +- 8 files changed, 130 insertions(+), 16 deletions(-) create mode 100644 tests/Doctrine/Tests/Models/Company/CompanyCar.php diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php index b4b692523..9c6167d59 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php @@ -186,7 +186,7 @@ class ClassMetadataFactory $class->setVersioned($parent->isVersioned); $class->setVersionField($parent->versionField); $class->setDiscriminatorMap($parent->discriminatorMap); - $class->resultColumnNames = $parent->resultColumnNames; + $class->setResultColumnNames($parent->resultColumnNames); } // Invoke driver @@ -227,6 +227,13 @@ class ClassMetadataFactory $this->_generateStaticSql($class); } + if ($parent) { + foreach ($visited as $parentClassName) { + $parentClass = $this->_loadedMetadata[$parentClassName]; + $parentClass->setResultColumnNames(array_merge($parentClass->resultColumnNames, $class->resultColumnNames)); + } + } + $this->_loadedMetadata[$className] = $class; $parent = $class; diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php index 2d987f099..1c745ba50 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php @@ -507,6 +507,11 @@ class ClassMetadataInfo { return $this->rootEntityName; } + + public function setResultColumnNames(array $resultColumnNames) + { + $this->resultColumnNames = $resultColumnNames; + } /** * Checks whether a field is part of the identifier/primary key field(s). diff --git a/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php b/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php index 2f9f6c5a7..692a223ac 100644 --- a/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php +++ b/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php @@ -309,30 +309,47 @@ class JoinedSubclassPersister extends StandardEntityPersister $this->_class->getQuotedDiscriminatorColumnName($this->_platform); } - $sql = 'SELECT ' . $columnList . ' FROM ' . $this->_class->getQuotedTableName($this->_platform) . ' ' . $baseTableAlias; - // INNER JOIN parent tables + $joinSql = ''; foreach ($this->_class->parentClasses as $parentClassName) { $parentClass = $this->_em->getClassMetadata($parentClassName); $tableAlias = $tableAliases[$parentClassName]; - $sql .= ' INNER JOIN ' . $parentClass->getQuotedTableName($this->_platform) . ' ' . $tableAlias . ' ON '; + $joinSql .= ' INNER JOIN ' . $parentClass->getQuotedTableName($this->_platform) . ' ' . $tableAlias . ' ON '; $first = true; foreach ($idColumns as $idColumn) { - if ($first) $first = false; else $sql .= ' AND '; - $sql .= $baseTableAlias . '.' . $idColumn . ' = ' . $tableAlias . '.' . $idColumn; + if ($first) $first = false; else $joinSql .= ' AND '; + $joinSql .= $baseTableAlias . '.' . $idColumn . ' = ' . $tableAlias . '.' . $idColumn; } } // OUTER JOIN sub tables foreach ($this->_class->subClasses as $subClassName) { - //FIXME: Add columns and foreign key columns of inherited classes to the select list $subClass = $this->_em->getClassMetadata($subClassName); $tableAlias = $tableAliases[$subClassName]; - $sql .= ' LEFT JOIN ' . $subClass->getQuotedTableName($this->_platform) . ' ' . $tableAlias . ' ON '; + + // Add subclass columns + foreach ($subClass->fieldMappings as $fieldName => $mapping) { + if (isset($mapping['inherited'])) { + continue; + } + $columnList .= ', ' . $tableAlias . '.' . $subClass->getQuotedColumnName($fieldName, $this->_platform); + } + + // Add join columns (foreign keys) + foreach ($subClass->associationMappings as $assoc2) { + if ($assoc2->isOwningSide && $assoc2->isOneToOne() && ! isset($subClass->inheritedAssociationFields[$assoc2->sourceFieldName])) { + foreach ($assoc2->targetToSourceKeyColumns as $srcColumn) { + $columnList .= ', ' . $tableAlias . '.' . $assoc2->getQuotedJoinColumnName($srcColumn, $this->_platform); + } + } + } + + // Add LEFT JOIN + $joinSql .= ' LEFT JOIN ' . $subClass->getQuotedTableName($this->_platform) . ' ' . $tableAlias . ' ON '; $first = true; foreach ($idColumns as $idColumn) { - if ($first) $first = false; else $sql .= ' AND '; - $sql .= $baseTableAlias . '.' . $idColumn . ' = ' . $tableAlias . '.' . $idColumn; + if ($first) $first = false; else $joinSql .= ' AND '; + $joinSql .= $baseTableAlias . '.' . $idColumn . ' = ' . $tableAlias . '.' . $idColumn; } } @@ -350,6 +367,9 @@ class JoinedSubclassPersister extends StandardEntityPersister $conditionSql .= ' = ?'; } - return $sql . ($conditionSql != '' ? ' WHERE ' . $conditionSql : ''); + return 'SELECT ' . $columnList + . ' FROM ' . $this->_class->getQuotedTableName($this->_platform) . ' ' . $baseTableAlias + . $joinSql + . ($conditionSql != '' ? ' WHERE ' . $conditionSql : ''); } } diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index 6e7b045c7..5103a650e 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -503,7 +503,6 @@ class SqlWalker implements TreeWalker // Add foreign key columns to SQL, if necessary if ($addMetaColumns) { $sqlTableAlias = $this->getSqlTableAlias($class->primaryTable['name'], $dqlAlias); - foreach ($class->associationMappings as $assoc) { if ($assoc->isOwningSide && $assoc->isOneToOne()) { foreach ($assoc->targetToSourceKeyColumns as $srcColumn) { @@ -817,13 +816,14 @@ class SqlWalker implements TreeWalker $this->_rsm->addFieldResult($dqlAlias, $columnAlias, $fieldName); } - // Add any additional fields of subclasses (not inherited fields) + // Add any additional fields of subclasses (excluding inherited fields) // 1) on Single Table Inheritance: always, since its marginal overhead // 2) on Class Table Inheritance only if partial objects are disallowed, // since it requires outer joining subtables. if ($class->isInheritanceTypeSingleTable() || ! $this->_query->getHint(Query::HINT_FORCE_PARTIAL_LOAD)) { foreach ($class->subClasses as $subClassName) { $subClass = $this->_em->getClassMetadata($subClassName); + $sqlTableAlias = $this->getSqlTableAlias($subClass->primaryTable['name'], $dqlAlias); foreach ($subClass->fieldMappings as $fieldName => $mapping) { if (isset($mapping['inherited'])) { continue; @@ -831,7 +831,6 @@ class SqlWalker implements TreeWalker if ($beginning) $beginning = false; else $sql .= ', '; - $sqlTableAlias = $this->getSqlTableAlias($subClass->primaryTable['name'], $dqlAlias); $columnAlias = $this->getSqlColumnAlias($mapping['columnName']); $sql .= $sqlTableAlias . '.' . $subClass->getQuotedColumnName($fieldName, $this->_platform) . ' AS ' . $columnAlias; @@ -839,6 +838,20 @@ class SqlWalker implements TreeWalker $columnAlias = $this->_platform->getSqlResultCasing($columnAlias); $this->_rsm->addFieldResult($dqlAlias, $columnAlias, $fieldName); } + + // Add join columns (foreign keys) of the subclass + //TODO: Probably better do this in walkSelectClause to honor the INCLUDE_META_COLUMNS hint + foreach ($subClass->associationMappings as $fieldName => $assoc) { + if ($assoc->isOwningSide && $assoc->isOneToOne() && ! isset($subClass->inheritedAssociationFields[$fieldName])) { + foreach ($assoc->targetToSourceKeyColumns as $srcColumn) { + if ($beginning) $beginning = false; else $sql .= ', '; + $columnAlias = $this->getSqlColumnAlias($srcColumn); + $sql .= $sqlTableAlias . '.' . $assoc->getQuotedJoinColumnName($srcColumn, $this->_platform) + . ' AS ' . $columnAlias; + $this->_rsm->addMetaResult($dqlAlias, $columnAlias, $srcColumn); + } + } + } } } } diff --git a/tests/Doctrine/Tests/Models/Company/CompanyCar.php b/tests/Doctrine/Tests/Models/Company/CompanyCar.php new file mode 100644 index 000000000..62f400486 --- /dev/null +++ b/tests/Doctrine/Tests/Models/Company/CompanyCar.php @@ -0,0 +1,33 @@ +brand = $brand; + } + + public function getId() { + return $this->id; + } + + public function getBrand() { + return $this->title; + } +} \ No newline at end of file diff --git a/tests/Doctrine/Tests/Models/Company/CompanyManager.php b/tests/Doctrine/Tests/Models/Company/CompanyManager.php index cde06ce6c..0c34fca3c 100644 --- a/tests/Doctrine/Tests/Models/Company/CompanyManager.php +++ b/tests/Doctrine/Tests/Models/Company/CompanyManager.php @@ -12,6 +12,12 @@ class CompanyManager extends CompanyEmployee * @Column(type="string", length="250") */ private $title; + + /** + * @OneToOne(targetEntity="CompanyCar", cascade={"persist"}) + * @JoinColumn(name="car_id", referencedColumnName="id") + */ + private $car; public function getTitle() { return $this->title; @@ -20,4 +26,12 @@ class CompanyManager extends CompanyEmployee public function setTitle($title) { $this->title = $title; } + + public function getCar() { + return $this->car; + } + + public function setCar(CompanyCar $car) { + $this->car = $car; + } } \ No newline at end of file diff --git a/tests/Doctrine/Tests/ORM/Functional/ClassTableInheritanceTest.php b/tests/Doctrine/Tests/ORM/Functional/ClassTableInheritanceTest.php index 4c744ad92..d45600605 100644 --- a/tests/Doctrine/Tests/ORM/Functional/ClassTableInheritanceTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/ClassTableInheritanceTest.php @@ -10,7 +10,8 @@ use Doctrine\Tests\Models\Company\CompanyPerson, Doctrine\Tests\Models\Company\CompanyOrganization, Doctrine\Tests\Models\Company\CompanyEvent, Doctrine\Tests\Models\Company\CompanyAuction, - Doctrine\Tests\Models\Company\CompanyRaffle; + Doctrine\Tests\Models\Company\CompanyRaffle, + Doctrine\Tests\Models\Company\CompanyCar; /** * Functional tests for the Class Table Inheritance mapping strategy. @@ -110,6 +111,27 @@ class ClassTableInheritanceTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->assertTrue(is_numeric($manager->getId())); } + public function testFindOnBaseClass() { + $manager = new CompanyManager; + $manager->setName('Roman S. Borschel'); + $manager->setSalary(100000); + $manager->setDepartment('IT'); + $manager->setTitle('CTO'); + $this->_em->persist($manager); + $this->_em->flush(); + + $this->_em->clear(); + + $person = $this->_em->find('Doctrine\Tests\Models\Company\CompanyPerson', $manager->getId()); + + $this->assertTrue($person instanceof CompanyManager); + $this->assertEquals('Roman S. Borschel', $person->getName()); + $this->assertEquals(100000, $person->getSalary()); + $this->assertEquals('CTO', $person->getTitle()); + $this->assertTrue(is_numeric($person->getId())); + //$this->assertTrue($person->getCar() instanceof CompanyCar); + } + public function testSelfReferencingOneToOne() { $manager = new CompanyManager; $manager->setName('John Smith'); diff --git a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php index 6ca3c8bdb..161be88c4 100644 --- a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php +++ b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php @@ -308,7 +308,7 @@ class SelectSqlGenerationTest extends \Doctrine\Tests\OrmTestCase $this->_em->getClassMetadata(get_class($person))->setIdentifierValues($person, 101); $q3->setParameter('param', $person); $this->assertEquals( - 'SELECT c0_.id AS id0, c0_.name AS name1, c1_.title AS title2, c2_.salary AS salary3, c2_.department AS department4, c0_.discr AS discr5, c0_.spouse_id AS spouse_id6 FROM company_persons c0_ LEFT JOIN company_managers c1_ ON c0_.id = c1_.id LEFT JOIN company_employees c2_ ON c0_.id = c2_.id WHERE EXISTS (SELECT 1 FROM company_persons_friends c3_ INNER JOIN company_persons c4_ ON c3_.person_id = c0_.id WHERE c3_.friend_id = c4_.id AND c4_.id = ?)', + 'SELECT c0_.id AS id0, c0_.name AS name1, c1_.title AS title2, c1_.car_id AS car_id3, c2_.salary AS salary4, c2_.department AS department5, c0_.discr AS discr6, c0_.spouse_id AS spouse_id7 FROM company_persons c0_ LEFT JOIN company_managers c1_ ON c0_.id = c1_.id LEFT JOIN company_employees c2_ ON c0_.id = c2_.id WHERE EXISTS (SELECT 1 FROM company_persons_friends c3_ INNER JOIN company_persons c4_ ON c3_.person_id = c0_.id WHERE c3_.friend_id = c4_.id AND c4_.id = ?)', $q3->getSql() ); }