From 4a385341509ea6e562d3487d8c7ae714cc6a3a13 Mon Sep 17 00:00:00 2001 From: Guilherme Blanco Date: Fri, 16 Aug 2013 00:07:06 -0400 Subject: [PATCH] Fixed DDC-2235. --- .../Query/AST/RangeVariableDeclaration.php | 15 +++- lib/Doctrine/ORM/Query/Parser.php | 19 +++-- lib/Doctrine/ORM/Query/SqlWalker.php | 32 +++++-- .../ORM/Query/SelectSqlGenerationTest.php | 83 ++++++++++++++++++- 4 files changed, 132 insertions(+), 17 deletions(-) diff --git a/lib/Doctrine/ORM/Query/AST/RangeVariableDeclaration.php b/lib/Doctrine/ORM/Query/AST/RangeVariableDeclaration.php index 72a2ea9aa..0ca5274d1 100644 --- a/lib/Doctrine/ORM/Query/AST/RangeVariableDeclaration.php +++ b/lib/Doctrine/ORM/Query/AST/RangeVariableDeclaration.php @@ -41,13 +41,20 @@ class RangeVariableDeclaration extends Node public $aliasIdentificationVariable; /** - * @param string $abstractSchemaName - * @param string $aliasIdentificationVar + * @var boolean */ - public function __construct($abstractSchemaName, $aliasIdentificationVar) + public $isRoot; + + /** + * @param string $abstractSchemaName + * @param string $aliasIdentificationVar + * @param boolean $isRoot + */ + public function __construct($abstractSchemaName, $aliasIdentificationVar, $isRoot = true) { - $this->abstractSchemaName = $abstractSchemaName; + $this->abstractSchemaName = $abstractSchemaName; $this->aliasIdentificationVariable = $aliasIdentificationVar; + $this->isRoot = $isRoot; } /** diff --git a/lib/Doctrine/ORM/Query/Parser.php b/lib/Doctrine/ORM/Query/Parser.php index a82d9d82f..9a3baad48 100644 --- a/lib/Doctrine/ORM/Query/Parser.php +++ b/lib/Doctrine/ORM/Query/Parser.php @@ -1544,6 +1544,9 @@ class Parser public function IdentificationVariableDeclaration() { $rangeVariableDeclaration = $this->RangeVariableDeclaration(); + + $rangeVariableDeclaration->isRoot = true; + $indexBy = $this->lexer->isNextToken(Lexer::T_INDEX) ? $this->IndexBy() : null; $joins = array(); @@ -1622,15 +1625,19 @@ class Parser $this->match(Lexer::T_JOIN); $next = $this->lexer->glimpse(); - $joinDeclaration = ($next['type'] === Lexer::T_DOT) - ? $this->JoinAssociationDeclaration() - : $this->RangeVariableDeclaration(); + $joinDeclaration = ($next['type'] === Lexer::T_DOT) ? $this->JoinAssociationDeclaration() : $this->RangeVariableDeclaration(); + $adhocConditions = $this->lexer->isNextToken(Lexer::T_WITH); + $join = new AST\Join($joinType, $joinDeclaration); - // Create AST node - $join = new AST\Join($joinType, $joinDeclaration); + // Describe non-root join declaration + if ($joinDeclaration instanceof AST\RangeVariableDeclaration) { + $joinDeclaration->isRoot = false; + + $adhocConditions = true; + } // Check for ad-hoc Join conditions - if ($this->lexer->isNextToken(Lexer::T_WITH) || $joinDeclaration instanceof AST\RangeVariableDeclaration) { + if ($adhocConditions) { $this->match(Lexer::T_WITH); $join->conditionalExpression = $this->ConditionalExpression(); diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index 790ab3f6d..157bbf618 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -842,7 +842,9 @@ class SqlWalker implements TreeWalker $class = $this->em->getClassMetadata($rangeVariableDeclaration->abstractSchemaName); $dqlAlias = $rangeVariableDeclaration->aliasIdentificationVariable; - $this->rootAliases[] = $dqlAlias; + if ($rangeVariableDeclaration->isRoot) { + $this->rootAliases[] = $dqlAlias; + } $sql = $this->quoteStrategy->getTableName($class,$this->platform) . ' ' . $this->getSQLTableAlias($class->getTableName(), $dqlAlias); @@ -1066,13 +1068,33 @@ class SqlWalker implements TreeWalker switch (true) { case ($joinDeclaration instanceof \Doctrine\ORM\Query\AST\RangeVariableDeclaration): - $class = $this->em->getClassMetadata($joinDeclaration->abstractSchemaName); - $condExprConjunction = $class->isInheritanceTypeJoined() && $joinType != AST\Join::JOIN_TYPE_LEFT && $joinType != AST\Join::JOIN_TYPE_LEFTOUTER + $class = $this->em->getClassMetadata($joinDeclaration->abstractSchemaName); + $dqlAlias = $joinDeclaration->aliasIdentificationVariable; + $tableAlias = $this->getSQLTableAlias($class->table['name'], $dqlAlias); + $condition = '(' . $this->walkConditionalExpression($join->conditionalExpression) . ')'; + $condExprConjunction = ($class->isInheritanceTypeJoined() && $joinType != AST\Join::JOIN_TYPE_LEFT && $joinType != AST\Join::JOIN_TYPE_LEFTOUTER) ? ' AND ' : ' ON '; - $sql .= $this->walkRangeVariableDeclaration($joinDeclaration) - . $condExprConjunction . '(' . $this->walkConditionalExpression($join->conditionalExpression) . ')'; + $sql .= $this->walkRangeVariableDeclaration($joinDeclaration); + + $conditions = array($condition); + + // Apply remaining inheritance restrictions + $discrSql = $this->_generateDiscriminatorColumnConditionSQL(array($dqlAlias)); + + if ($discrSql) { + $conditions[] = $discrSql; + } + + // Apply the filters + $filterExpr = $this->generateFilterConditionSQL($class, $tableAlias); + + if ($filterExpr) { + $conditions[] = $filterExpr; + } + + $sql .= $condExprConjunction . implode(' AND ', $conditions); 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 8c112fe43..f36d3ef75 100644 --- a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php +++ b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php @@ -155,12 +155,12 @@ class SelectSqlGenerationTest extends \Doctrine\Tests\OrmTestCase { $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)' + '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 c4_ ON c2_.id = c4_.id INNER JOIN company_persons c3_ ON c2_.id = c3_.id AND (c0_.id = c3_.id)' ); $this->assertSqlGeneration( 'SELECT e FROM Doctrine\Tests\Models\Company\CompanyEmployee e LEFT 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 LEFT JOIN company_managers c2_ INNER JOIN company_employees c3_ ON c2_.id = c3_.id INNER JOIN company_persons c4_ ON c2_.id = c4_.id ON (c0_.id = c4_.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 LEFT JOIN company_managers c2_ INNER JOIN company_employees c4_ ON c2_.id = c4_.id INNER JOIN company_persons c3_ ON c2_.id = c3_.id ON (c0_.id = c3_.id)' ); } @@ -2052,6 +2052,85 @@ class SelectSqlGenerationTest extends \Doctrine\Tests\OrmTestCase ); } + /** + * @group DDC-2235 + */ + public function testSingleTableInheritanceLeftJoinWithCondition() + { + // Regression test for the bug + $this->assertSqlGeneration( + 'SELECT c FROM Doctrine\Tests\Models\Company\CompanyEmployee e LEFT JOIN Doctrine\Tests\Models\Company\CompanyContract c WITH c.salesPerson = e.id', + "SELECT c0_.id AS id0, c0_.completed AS completed1, c0_.fixPrice AS fixPrice2, c0_.hoursWorked AS hoursWorked3, c0_.pricePerHour AS pricePerHour4, c0_.maxPrice AS maxPrice5, c0_.discr AS discr6 FROM company_employees c1_ INNER JOIN company_persons c2_ ON c1_.id = c2_.id LEFT JOIN company_contracts c0_ ON (c0_.salesPerson_id = c2_.id) AND c0_.discr IN ('fix', 'flexible', 'flexultra')" + ); + } + + /** + * @group DDC-2235 + */ + public function testSingleTableInheritanceLeftJoinWithConditionAndWhere() + { + // Ensure other WHERE predicates are passed through to the main WHERE clause + $this->assertSqlGeneration( + 'SELECT c FROM Doctrine\Tests\Models\Company\CompanyEmployee e LEFT JOIN Doctrine\Tests\Models\Company\CompanyContract c WITH c.salesPerson = e.id WHERE e.salary > 1000', + "SELECT c0_.id AS id0, c0_.completed AS completed1, c0_.fixPrice AS fixPrice2, c0_.hoursWorked AS hoursWorked3, c0_.pricePerHour AS pricePerHour4, c0_.maxPrice AS maxPrice5, c0_.discr AS discr6 FROM company_employees c1_ INNER JOIN company_persons c2_ ON c1_.id = c2_.id LEFT JOIN company_contracts c0_ ON (c0_.salesPerson_id = c2_.id) AND c0_.discr IN ('fix', 'flexible', 'flexultra') WHERE c1_.salary > 1000" + ); + } + + /** + * @group DDC-2235 + */ + public function testSingleTableInheritanceInnerJoinWithCondition() + { + // Test inner joins too + $this->assertSqlGeneration( + 'SELECT c FROM Doctrine\Tests\Models\Company\CompanyEmployee e INNER JOIN Doctrine\Tests\Models\Company\CompanyContract c WITH c.salesPerson = e.id', + "SELECT c0_.id AS id0, c0_.completed AS completed1, c0_.fixPrice AS fixPrice2, c0_.hoursWorked AS hoursWorked3, c0_.pricePerHour AS pricePerHour4, c0_.maxPrice AS maxPrice5, c0_.discr AS discr6 FROM company_employees c1_ INNER JOIN company_persons c2_ ON c1_.id = c2_.id INNER JOIN company_contracts c0_ ON (c0_.salesPerson_id = c2_.id) AND c0_.discr IN ('fix', 'flexible', 'flexultra')" + ); + } + + /** + * @group DDC-2235 + */ + public function testSingleTableInheritanceLeftJoinNonAssociationWithConditionAndWhere() + { + // Test that the discriminator IN() predicate is still added into + // the where clause when not joining onto that table + $this->assertSqlGeneration( + 'SELECT c FROM Doctrine\Tests\Models\Company\CompanyContract c LEFT JOIN Doctrine\Tests\Models\Company\CompanyEmployee e WITH e.id = c.salesPerson WHERE c.completed = true', + "SELECT c0_.id AS id0, c0_.completed AS completed1, c0_.fixPrice AS fixPrice2, c0_.hoursWorked AS hoursWorked3, c0_.pricePerHour AS pricePerHour4, c0_.maxPrice AS maxPrice5, c0_.discr AS discr6 FROM company_contracts c0_ LEFT JOIN company_employees c1_ INNER JOIN company_persons c2_ ON c1_.id = c2_.id ON (c2_.id = c0_.salesPerson_id) WHERE (c0_.completed = 1) AND c0_.discr IN ('fix', 'flexible', 'flexultra')" + ); + } + + /** + * @group DDC-2235 + */ + public function testSingleTableInheritanceJoinCreatesOnCondition() + { + // Test that the discriminator IN() predicate is still added + // into the where clause when not joining onto a single table inheritance entity + // via a join association + $this->assertSqlGeneration( + 'SELECT c FROM Doctrine\Tests\Models\Company\CompanyContract c JOIN c.salesPerson s WHERE c.completed = true', + "SELECT c0_.id AS id0, c0_.completed AS completed1, c0_.fixPrice AS fixPrice2, c0_.hoursWorked AS hoursWorked3, c0_.pricePerHour AS pricePerHour4, c0_.maxPrice AS maxPrice5, c0_.discr AS discr6 FROM company_contracts c0_ INNER JOIN company_employees c1_ ON c0_.salesPerson_id = c1_.id LEFT JOIN company_persons c2_ ON c1_.id = c2_.id WHERE (c0_.completed = 1) AND c0_.discr IN ('fix', 'flexible', 'flexultra')" + ); + } + + /** + * @group DDC-2235 + */ + public function testSingleTableInheritanceCreatesOnConditionAndWhere() + { + // Test that when joining onto an entity using single table inheritance via + // a join association that the discriminator IN() predicate is placed + // into the ON clause of the join + $this->assertSqlGeneration( + 'SELECT e, COUNT(c) FROM Doctrine\Tests\Models\Company\CompanyEmployee e JOIN e.contracts c WHERE e.department = :department', + "SELECT c0_.id AS id0, c0_.name AS name1, c1_.salary AS salary2, c1_.department AS department3, c1_.startDate AS startDate4, COUNT(c2_.id) AS sclr5, c0_.discr AS discr6 FROM company_employees c1_ INNER JOIN company_persons c0_ ON c1_.id = c0_.id INNER JOIN company_contract_employees c3_ ON c1_.id = c3_.employee_id INNER JOIN company_contracts c2_ ON c2_.id = c3_.contract_id AND c2_.discr IN ('fix', 'flexible', 'flexultra') WHERE c1_.department = ?", + array(), + array('department' => 'foobar') + ); + } + /** * @group DDC-1858 */