From c6810861ca3cbcfc846a50c59e06681d84604332 Mon Sep 17 00:00:00 2001 From: Strate Date: Sun, 22 Dec 2013 21:02:14 +0400 Subject: [PATCH 1/3] Fix applying ON/WITH conditions to first join in Class Table Inheritance --- lib/Doctrine/ORM/Query/SqlWalker.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index 37f970843..549fe8d2d 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -982,6 +982,11 @@ class SqlWalker implements TreeWalker break; } + // FIXME: these should either be nested or all forced to be left joins (DDC-XXX) + if ($targetClass->isInheritanceTypeJoined()) { + $sql .= $this->_generateClassTableInheritanceJoins($targetClass, $joinedDqlAlias); + } + // Handle WITH clause if ($condExpr !== null) { // Phase 2 AST optimization: Skip processing of ConditionalExpression @@ -989,11 +994,6 @@ class SqlWalker implements TreeWalker $sql .= ' AND (' . $this->walkConditionalExpression($condExpr) . ')'; } - // FIXME: these should either be nested or all forced to be left joins (DDC-XXX) - if ($targetClass->isInheritanceTypeJoined()) { - $sql .= $this->_generateClassTableInheritanceJoins($targetClass, $joinedDqlAlias); - } - // Apply the indexes if ($indexBy) { // For Many-To-One or One-To-One associations this obviously makes no sense, but is ignored silently. From 41ec5fd56d6402096ab8e44fd16a1182c30055fd Mon Sep 17 00:00:00 2001 From: Strate Date: Sun, 19 Jan 2014 20:56:24 +0400 Subject: [PATCH 2/3] Fix applying ON/WITH conditions to first join in Class Table Inheritance Now we build nested joins for CTI when using ON/WITH clause. --- lib/Doctrine/ORM/Query/SqlWalker.php | 36 +++++++++++++------ .../ORM/Query/SelectSqlGenerationTest.php | 2 +- 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index f2425dd90..1a63570f0 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -895,6 +895,8 @@ class SqlWalker implements TreeWalker } } + $targetTableJoin = null; + // This condition is not checking ClassMetadata::MANY_TO_ONE, because by definition it cannot // be the owning side and previously we ensured that $assoc is always the owning side of the associations. // The owning side is necessary at this point because only it contains the JoinColumn information. @@ -929,7 +931,10 @@ class SqlWalker implements TreeWalker $conditions[] = $filterExpr; } - $sql .= $targetTableName . ' ' . $targetTableAlias . ' ON ' . implode(' AND ', $conditions); + $targetTableJoin = array( + 'table' => $targetTableName . ' ' . $targetTableAlias, + 'condition' => implode(' AND ', $conditions), + ); break; case ($assoc['type'] == ClassMetadata::MANY_TO_MANY): @@ -981,20 +986,29 @@ class SqlWalker implements TreeWalker $conditions[] = $filterExpr; } - $sql .= $targetTableName . ' ' . $targetTableAlias . ' ON ' . implode(' AND ', $conditions); + $targetTableJoin = array( + 'table' => $targetTableName . ' ' . $targetTableAlias, + 'condition' => implode(' AND ', $conditions), + ); break; } - // FIXME: these should either be nested or all forced to be left joins (DDC-XXX) - if ($targetClass->isInheritanceTypeJoined()) { - $sql .= $this->_generateClassTableInheritanceJoins($targetClass, $joinedDqlAlias); - } - // Handle WITH clause - if ($condExpr !== null) { - // Phase 2 AST optimization: Skip processing of ConditionalExpression - // if only one ConditionalTerm is defined - $sql .= ' AND (' . $this->walkConditionalExpression($condExpr) . ')'; + $withCondition = (null !== $condExpr) ? ('(' . $this->walkConditionalExpression($condExpr) . ')') : ''; + + if ($targetClass->isInheritanceTypeJoined()) { + $ctiJoins = $this->_generateClassTableInheritanceJoins($targetClass, $joinedDqlAlias); + // If we have WITH condition, we need to build nested joins for target class table and cti joins + if ($withCondition) { + $sql .= '(' . $targetTableJoin['table'] . $ctiJoins . ') ON ' . $targetTableJoin['condition'] . ' AND ' . $withCondition; + } else { + $sql .= $targetTableJoin['table'] . ' ON ' . $targetTableJoin['condition'] . $ctiJoins; + } + } else { + $sql .= $targetTableJoin['table'] . ' ON ' . $targetTableJoin['condition']; + if ($withCondition) { + $sql .= ' AND ' . $withCondition; + } } // Apply the indexes diff --git a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php index f0afcc798..83c34ce36 100644 --- a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php +++ b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php @@ -2065,7 +2065,7 @@ class SelectSqlGenerationTest extends \Doctrine\Tests\OrmTestCase { $this->assertSqlGeneration( 'SELECT e.id FROM Doctrine\Tests\Models\Company\CompanyOrganization o JOIN o.events e WITH e.id = ?1', - 'SELECT c0_.id AS id0 FROM company_organizations c1_ INNER JOIN company_events c0_ ON c1_.id = c0_.org_id AND (c0_.id = ?) LEFT JOIN company_auctions c2_ ON c0_.id = c2_.id LEFT JOIN company_raffles c3_ ON c0_.id = c3_.id', + 'SELECT c0_.id AS id0 FROM company_organizations c1_ INNER JOIN (company_events c0_ LEFT JOIN company_auctions c2_ ON c0_.id = c2_.id LEFT JOIN company_raffles c3_ ON c0_.id = c3_.id) ON c1_.id = c0_.org_id AND (c0_.id = ?)', array(Query::HINT_FORCE_PARTIAL_LOAD => false) ); } From 04e6061584e2691c69e62b43e8ad18a2801773aa Mon Sep 17 00:00:00 2001 From: Strate Date: Mon, 20 Jan 2014 20:06:53 +0400 Subject: [PATCH 3/3] Added an exception when invalid case. Fixes after code review. --- lib/Doctrine/ORM/Query/SqlWalker.php | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index 1a63570f0..2482b7611 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -991,24 +991,28 @@ class SqlWalker implements TreeWalker 'condition' => implode(' AND ', $conditions), ); break; + + default: + throw new \BadMethodCallException('Type of association must be one of *_TO_ONE or MANY_TO_MANY'); } // Handle WITH clause - $withCondition = (null !== $condExpr) ? ('(' . $this->walkConditionalExpression($condExpr) . ')') : ''; + $withCondition = (null === $condExpr) ? '' : ('(' . $this->walkConditionalExpression($condExpr) . ')'); if ($targetClass->isInheritanceTypeJoined()) { $ctiJoins = $this->_generateClassTableInheritanceJoins($targetClass, $joinedDqlAlias); // If we have WITH condition, we need to build nested joins for target class table and cti joins if ($withCondition) { - $sql .= '(' . $targetTableJoin['table'] . $ctiJoins . ') ON ' . $targetTableJoin['condition'] . ' AND ' . $withCondition; + $sql .= '(' . $targetTableJoin['table'] . $ctiJoins . ') ON ' . $targetTableJoin['condition']; } else { $sql .= $targetTableJoin['table'] . ' ON ' . $targetTableJoin['condition'] . $ctiJoins; } } else { $sql .= $targetTableJoin['table'] . ' ON ' . $targetTableJoin['condition']; - if ($withCondition) { - $sql .= ' AND ' . $withCondition; - } + } + + if ($withCondition) { + $sql .= ' AND ' . $withCondition; } // Apply the indexes