Don't include duplicate keys when eagerly loading

When eagerly loading associations with secondary queries, the ORM should
only include each value once. This will only work in the simple primary
key case, as scanning through composite key results can be pretty
expensive.

The main goal/advantage of this is that for large applications we can
avoid parameter limitations in SQLServer/PDO for more cases.

Refs #5778

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,28 @@ 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);
+    }
+
 }