From 3994b80aa4c31e7ca4253f1fef1c2d9a72789c3f Mon Sep 17 00:00:00 2001
From: Alexander <iam.asm89@gmail.com>
Date: Mon, 31 Oct 2011 21:36:55 +0100
Subject: [PATCH 1/3] Fix TODO: Inner join when all join columns are NOT
 nullable.

---
 .../ORM/Persisters/BasicEntityPersister.php   | 23 +++++++++++++++++--
 .../Functional/OneToOneEagerLoadingTest.php   |  3 ++-
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php
index 0379ccf56..78c79c283 100644
--- a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php
+++ b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php
@@ -1002,10 +1002,10 @@ class BasicEntityPersister
                     }
                 }
                 
-                $this->_selectJoinSql .= ' LEFT JOIN'; // TODO: Inner join when all join columns are NOT nullable.
                 $first = true;
                 
                 if ($assoc['isOwningSide']) {
+                    $this->_selectJoinSql .= $this->getJoinSQLForJoinColumns($assoc['joinColumns']);
                     $this->_selectJoinSql .= ' ' . $eagerEntity->getQuotedTableName($this->_platform) . ' ' . $this->_getSQLTableAlias($eagerEntity->name, $assocAlias) .' ON ';
                     
                     foreach ($assoc['sourceToTargetKeyColumns'] AS $sourceCol => $targetCol) {
@@ -1020,7 +1020,8 @@ class BasicEntityPersister
                 } else {
                     $eagerEntity = $this->_em->getClassMetadata($assoc['targetEntity']);
                     $owningAssoc = $eagerEntity->getAssociationMapping($assoc['mappedBy']);
-                    
+
+                    $this->_selectJoinSql .= $this->getJoinSQLForJoinColumns($owningAssoc['joinColumns']);
                     $this->_selectJoinSql .= ' ' . $eagerEntity->getQuotedTableName($this->_platform) . ' ' 
                                            . $this->_getSQLTableAlias($eagerEntity->name, $assocAlias) . ' ON ';
 
@@ -1500,4 +1501,22 @@ class BasicEntityPersister
         
         return (bool) $this->_conn->fetchColumn($sql, $params);
     }
+
+    /**
+     * Generates the appropriate join SQL for the given join column.
+     *
+     * @param array $joinColumns The join columns definition of an association.
+     * @return string LEFT JOIN if one of the columns is nullable, INNER JOIN otherwise.
+     */
+    protected function getJoinSQLForJoinColumns($joinColumns)
+    {
+        // if one of the join columns is nullable, return left join
+        foreach($joinColumns as $joinColumn) {
+             if(isset($joinColumn['nullable']) && $joinColumn['nullable']){
+                 return ' LEFT JOIN ';
+             }
+        }
+
+        return ' INNER JOIN ';
+    }
 }
diff --git a/tests/Doctrine/Tests/ORM/Functional/OneToOneEagerLoadingTest.php b/tests/Doctrine/Tests/ORM/Functional/OneToOneEagerLoadingTest.php
index 044a17381..7b87046f9 100644
--- a/tests/Doctrine/Tests/ORM/Functional/OneToOneEagerLoadingTest.php
+++ b/tests/Doctrine/Tests/ORM/Functional/OneToOneEagerLoadingTest.php
@@ -130,6 +130,7 @@ class Train
     /**
      * Owning side
      * @OneToOne(targetEntity="TrainDriver", inversedBy="train", fetch="EAGER", cascade={"persist"})
+     * @JoinColumn(nullable=true)
      */
     public $driver;
     /**
@@ -195,4 +196,4 @@ class Waggon
     {
         $this->train = $train;
     }
-}
\ No newline at end of file
+}

From 0f938b8c1d37402a80c94c0b16c8d5020f8f314d Mon Sep 17 00:00:00 2001
From: Alexander <iam.asm89@gmail.com>
Date: Mon, 31 Oct 2011 21:53:46 +0100
Subject: [PATCH 2/3] Added tests for inner join generation with eager loading

---
 .../Functional/OneToOneEagerLoadingTest.php   | 28 +++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/tests/Doctrine/Tests/ORM/Functional/OneToOneEagerLoadingTest.php b/tests/Doctrine/Tests/ORM/Functional/OneToOneEagerLoadingTest.php
index 7b87046f9..f8760c6f6 100644
--- a/tests/Doctrine/Tests/ORM/Functional/OneToOneEagerLoadingTest.php
+++ b/tests/Doctrine/Tests/ORM/Functional/OneToOneEagerLoadingTest.php
@@ -115,6 +115,34 @@ class OneToOneEagerLoadingTest extends \Doctrine\Tests\OrmFunctionalTestCase
         $this->assertNotInstanceOf('Doctrine\ORM\Proxy\Proxy', $waggon->train);
         $this->assertNotNull($waggon->train);
     }
+
+    public function testEagerLoadWithNullableColumnsGeneratesLeftJoin()
+    {
+        $train = new Train();
+        $this->_em->persist($train);
+        $this->_em->flush();
+        $this->_em->clear();
+
+        $train = $this->_em->find(get_class($train), $train->id);
+        $this->assertEquals(
+            $this->_sqlLoggerStack->queries[$this->_sqlLoggerStack->currentQuery]['sql'],
+            "SELECT t0.id AS id1, t0.driver_id AS driver_id2, t3.id AS id4, t3.name AS name5 FROM Train t0 LEFT JOIN  TrainDriver t3 ON t0.driver_id = t3.id  WHERE t0.id = ?"
+        );
+    }
+
+    public function testEagerLoadWithNonNullableColumnsGeneratesInnerJoin()
+    {
+        $waggon = new Waggon();
+        $this->_em->persist($waggon);
+        $this->_em->flush();
+        $this->_em->clear();
+
+        $waggon = $this->_em->find(get_class($waggon), $waggon->id);
+        $this->assertEquals(
+            $this->_sqlLoggerStack->queries[$this->_sqlLoggerStack->currentQuery]['sql'],
+            "SELECT t0.id AS id1, t0.train_id AS train_id2, t3.id AS id4, t3.driver_id AS driver_id5 FROM Waggon t0 INNER JOIN  Train t3 ON t0.train_id = t3.id  WHERE t0.id = ?"
+        );
+    }
 }
 
 /**

From 22b3b46b61fa0cf647129fe2b52dd06549475dab Mon Sep 17 00:00:00 2001
From: Alexander <iam.asm89@gmail.com>
Date: Mon, 31 Oct 2011 22:08:40 +0100
Subject: [PATCH 3/3] Removed unnecessary spaces in generated SQL

---
 lib/Doctrine/ORM/Persisters/BasicEntityPersister.php | 12 ++++++------
 .../ORM/Functional/OneToOneEagerLoadingTest.php      |  8 ++++----
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php
index 78c79c283..b747155b3 100644
--- a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php
+++ b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php
@@ -1005,7 +1005,7 @@ class BasicEntityPersister
                 $first = true;
                 
                 if ($assoc['isOwningSide']) {
-                    $this->_selectJoinSql .= $this->getJoinSQLForJoinColumns($assoc['joinColumns']);
+                    $this->_selectJoinSql .= ' ' . $this->getJoinSQLForJoinColumns($assoc['joinColumns']);
                     $this->_selectJoinSql .= ' ' . $eagerEntity->getQuotedTableName($this->_platform) . ' ' . $this->_getSQLTableAlias($eagerEntity->name, $assocAlias) .' ON ';
                     
                     foreach ($assoc['sourceToTargetKeyColumns'] AS $sourceCol => $targetCol) {
@@ -1014,14 +1014,14 @@ class BasicEntityPersister
                         }
                         
                         $this->_selectJoinSql .= $this->_getSQLTableAlias($assoc['sourceEntity']) . '.' . $sourceCol . ' = ' 
-                                               . $this->_getSQLTableAlias($assoc['targetEntity'], $assocAlias) . '.' . $targetCol . ' ';
+                                               . $this->_getSQLTableAlias($assoc['targetEntity'], $assocAlias) . '.' . $targetCol;
                         $first = false;
                     }
                 } else {
                     $eagerEntity = $this->_em->getClassMetadata($assoc['targetEntity']);
                     $owningAssoc = $eagerEntity->getAssociationMapping($assoc['mappedBy']);
 
-                    $this->_selectJoinSql .= $this->getJoinSQLForJoinColumns($owningAssoc['joinColumns']);
+                    $this->_selectJoinSql .= ' ' . $this->getJoinSQLForJoinColumns($owningAssoc['joinColumns']);
                     $this->_selectJoinSql .= ' ' . $eagerEntity->getQuotedTableName($this->_platform) . ' ' 
                                            . $this->_getSQLTableAlias($eagerEntity->name, $assocAlias) . ' ON ';
 
@@ -1031,7 +1031,7 @@ class BasicEntityPersister
                         }
                         
                         $this->_selectJoinSql .= $this->_getSQLTableAlias($owningAssoc['sourceEntity'], $assocAlias) . '.' . $sourceCol . ' = ' 
-                                               . $this->_getSQLTableAlias($owningAssoc['targetEntity']) . '.' . $targetCol . ' ';
+                                               . $this->_getSQLTableAlias($owningAssoc['targetEntity']) . '.' . $targetCol;
                         $first = false;
                     }
                 }
@@ -1513,10 +1513,10 @@ class BasicEntityPersister
         // if one of the join columns is nullable, return left join
         foreach($joinColumns as $joinColumn) {
              if(isset($joinColumn['nullable']) && $joinColumn['nullable']){
-                 return ' LEFT JOIN ';
+                 return 'LEFT JOIN';
              }
         }
 
-        return ' INNER JOIN ';
+        return 'INNER JOIN';
     }
 }
diff --git a/tests/Doctrine/Tests/ORM/Functional/OneToOneEagerLoadingTest.php b/tests/Doctrine/Tests/ORM/Functional/OneToOneEagerLoadingTest.php
index f8760c6f6..5361aad45 100644
--- a/tests/Doctrine/Tests/ORM/Functional/OneToOneEagerLoadingTest.php
+++ b/tests/Doctrine/Tests/ORM/Functional/OneToOneEagerLoadingTest.php
@@ -125,8 +125,8 @@ class OneToOneEagerLoadingTest extends \Doctrine\Tests\OrmFunctionalTestCase
 
         $train = $this->_em->find(get_class($train), $train->id);
         $this->assertEquals(
-            $this->_sqlLoggerStack->queries[$this->_sqlLoggerStack->currentQuery]['sql'],
-            "SELECT t0.id AS id1, t0.driver_id AS driver_id2, t3.id AS id4, t3.name AS name5 FROM Train t0 LEFT JOIN  TrainDriver t3 ON t0.driver_id = t3.id  WHERE t0.id = ?"
+            "SELECT t0.id AS id1, t0.driver_id AS driver_id2, t3.id AS id4, t3.name AS name5 FROM Train t0 LEFT JOIN TrainDriver t3 ON t0.driver_id = t3.id WHERE t0.id = ?",
+            $this->_sqlLoggerStack->queries[$this->_sqlLoggerStack->currentQuery]['sql']
         );
     }
 
@@ -139,8 +139,8 @@ class OneToOneEagerLoadingTest extends \Doctrine\Tests\OrmFunctionalTestCase
 
         $waggon = $this->_em->find(get_class($waggon), $waggon->id);
         $this->assertEquals(
-            $this->_sqlLoggerStack->queries[$this->_sqlLoggerStack->currentQuery]['sql'],
-            "SELECT t0.id AS id1, t0.train_id AS train_id2, t3.id AS id4, t3.driver_id AS driver_id5 FROM Waggon t0 INNER JOIN  Train t3 ON t0.train_id = t3.id  WHERE t0.id = ?"
+            "SELECT t0.id AS id1, t0.train_id AS train_id2, t3.id AS id4, t3.driver_id AS driver_id5 FROM Waggon t0 INNER JOIN Train t3 ON t0.train_id = t3.id WHERE t0.id = ?",
+            $this->_sqlLoggerStack->queries[$this->_sqlLoggerStack->currentQuery]['sql']
         );
     }
 }