Merge pull request #5796 from cakephp/3.0-no-join-dupes

Don't include duplicate keys when eagerly loading

src/ORM/Association.php

@@ -827,7 +827,7 @@ abstract class Association
      * the source table.
      *
      * The required way of passing related source records is controlled by "strategy"
-     * By default the subquery strategy is used, which requires a query on the source
+     * When the subquery strategy is used it will require a query on the source table.
      * When using the select strategy, the list of primary keys will be used.
      *
      * Returns a closure that should be run for each record returned in a specific

src/ORM/EagerLoader.php

@@ -491,6 +491,7 @@ class EagerLoader
 
         $driver = $query->connection()->driver();
         list($collected, $statement) = $this->_collectKeys($external, $query, $statement);
+
         foreach ($external as $meta) {
             $contain = $meta->associations();
             $instance = $meta->instance();
@@ -501,8 +502,8 @@ class EagerLoader
             if ($requiresKeys && empty($collected[$alias])) {
                 continue;
             }
-
             $keys = isset($collected[$alias]) ? $collected[$alias] : null;
+
             $f = $instance->eagerLoader(
                 $config + [
                     'query' => $query,
@@ -642,14 +643,15 @@ class EagerLoader
         while ($result = $statement->fetch('assoc')) {
             foreach ($collectKeys as $parts) {
                 // Missed joins will have null in the results.
-                if ($parts[2] && !isset($result[$parts[1][0]])) {
+                if ($parts[2] === true && !isset($result[$parts[1][0]])) {
                     continue;
                 }
-                if ($parts[2]) {
+                if ($parts[2] === true) {
                     $keys[$parts[0]][] = $result[$parts[1][0]];
                     continue;
                 }
 
+                // Handle composite keys.
                 $collected = [];
                 foreach ($parts[1] as $key) {
                     $collected[] = $result[$key];
@@ -658,6 +660,12 @@ class EagerLoader
             }
         }
 
+        foreach ($collectKeys as $parts) {
+            if ($parts[2] === true && isset($keys[$parts[0]])) {
+                $keys[$parts[0]] = array_unique($keys[$parts[0]]);
+            }
+        }
+
         $statement->rewind();
         return $keys;
     }

tests/TestCase/ORM/QueryRegressionTest.php

@@ -740,4 +740,27 @@ class QueryRegressionTest extends TestCase
         $comments = TableRegistry::get('Comments');
         $comments->find()->contain('Deprs')->all();
     }
+
+    /**
+     * Tests that HasMany associations don't use duplicate PK values.
+     *
+     * @return void
+     */
+    public function testHasManyEagerLoadingUniqueKey()
+    {
+        $table = TableRegistry::get('ArticlesTags');
+        $table->belongsTo('Articles', [
+            'strategy' => 'select'
+        ]);
+
+        $result = $table->find()
+            ->contain(['Articles' => function ($q) {
+                $result = $q->sql();
+                $this->assertNotContains(':c2', $result, 'Only 2 bindings as there are only 2 rows.');
+                $this->assertNotContains(':c3', $result, 'Only 2 bindings as there are only 2 rows.');
+                return $q;
+            }])
+            ->toArray();
+        $this->assertNotEmpty($result[0]->article);
+    }
 }