From 9e916a2893d0fa9377d2ad8582d6b2d40070ce67 Mon Sep 17 00:00:00 2001 From: Gordon Stratton Date: Mon, 5 Nov 2012 01:19:25 -0800 Subject: [PATCH] Fix for invalid 'double-ON' SQL generation with entity inheritance type JOINED. In SqlWalker::walkJoin(), SqlWalker::walkRangeVariableDeclaration() can be called which may produce an 'ON' clause if the entity inheritance type is JOINED. As walkJoin() may then produce another ON clause, this results in invalid SQL (e.g. '... ON foo = bar ON (baz = quux) ...' when the inheritance type is JOINED. This adds a test and a fix for the problem, by checking for an inheritance type of JOINED in walkJoin() and using AND instead of ON in the appropriate place. It seems like this part of the code is begging to be refactored. This is my first foray into Doctrine internals and can't see a way to do this without stomping all over the rest of the code, but this section seems ripe for cleanup by somebody who is familiar. --- lib/Doctrine/ORM/Query/SqlWalker.php | 7 ++++++- .../Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php | 8 ++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index 26e31bdd7..1ad4fc07b 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -1032,8 +1032,13 @@ class SqlWalker implements TreeWalker switch (true) { case ($joinDeclaration instanceof \Doctrine\ORM\Query\AST\RangeVariableDeclaration): + $class = $this->em->getClassMetadata($joinDeclaration->abstractSchemaName); + $condExprConjunction = $class->isInheritanceTypeJoined() + ? ' AND ' + : ' ON '; + $sql .= $this->walkRangeVariableDeclaration($joinDeclaration) - . ' ON (' . $this->walkConditionalExpression($join->conditionalExpression) . ')'; + . $condExprConjunction . '(' . $this->walkConditionalExpression($join->conditionalExpression) . ')'; break; case ($joinDeclaration instanceof \Doctrine\ORM\Query\AST\JoinAssociationDeclaration): diff --git a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php index 351615f42..9ce5cc30d 100644 --- a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php +++ b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php @@ -143,6 +143,14 @@ class SelectSqlGenerationTest extends \Doctrine\Tests\OrmTestCase ); } + public function testSupportsJoinOnMultipleComponentsWithJoinedInheritanceType() + { + $this->assertSqlGeneration( + 'SELECT e FROM Doctrine\Tests\Models\Company\CompanyEmployee e JOIN Doctrine\Tests\Models\Company\CompanyManager m WITH e.id = m.id', + 'SELECT c0_.id AS id0, c0_.name AS name1, c1_.salary AS salary2, c1_.department AS department3, c1_.startDate AS startDate4, c0_.discr AS discr5 FROM company_employees c1_ INNER JOIN company_persons c0_ ON c1_.id = c0_.id INNER JOIN company_managers c2_ INNER JOIN company_employees c3_ ON c2_.id = c3_.id INNER JOIN company_persons c4_ ON c2_.id = c4_.id AND (c0_.id = c4_.id)' + ); + } + public function testSupportsSelectWithCollectionAssociationJoin() { $this->assertSqlGeneration(