Optimize count queries

src/ORM.php

@@ -402,7 +402,7 @@ public function queueDelete($entity, int $mode = TransactionInterface::MODE_CASC
      *
      * @return array
      */
-    protected function getIndexes(string $role): array
+    public function getIndexes(string $role): array
     {
         if (isset($this->indexes[$role])) {
             return $this->indexes[$role];

src/Select.php

@@ -65,7 +65,7 @@ public function __construct(ORMInterface $orm, string $role)
     {
         $this->orm = $orm;
         $this->loader = new RootLoader($orm, $this->orm->resolveRole($role));
-        $this->builder = new QueryBuilder($this->getLoader()->getQuery(), $this->loader);
+        $this->builder = new QueryBuilder($this->loader->getQuery(), $this->loader);
     }
 
     /**
@@ -167,18 +167,13 @@ public function wherePK($id): self
     /**
      * Attention, column will be quoted by driver!
      *
-     * @param string|null $column When column is null DISTINCT(PK) will be generated.
+     * @param string|null $column When column is null PK or DISTINCT(PK) will be generated.
      *
      * @return int
      */
     public function count(string $column = null): int
     {
-        if ($column === null) {
-            // @tuneyourserver solves the issue with counting on queries with joins.
-            $column = sprintf('DISTINCT(%s)', $this->loader->getPK());
-        }
-
-        return (int) $this->__call('count', [$column]);
+        return (int) $this->__call('count', [$column ?? $this->loader->getCountField()]);
     }
 
     /**
@@ -441,14 +436,4 @@ public function sqlStatement(): string
     {
         return $this->buildQuery()->sqlStatement();
     }
-
-    /**
-     * Return base loader associated with the selector.
-     *
-     * @return RootLoader
-     */
-    private function getLoader(): RootLoader
-    {
-        return $this->loader;
-    }
 }

src/Select/AbstractLoader.php

@@ -259,6 +259,23 @@ public function loadData(AbstractNode $node): void
      */
     abstract public function isLoaded(): bool;
 
+    /**
+     * Indicates that query can load multiple joined rows,
+     * which possibly can lead to data duplication.
+     *
+     * @return bool
+     */
+    public function isDataDuplicationPossible(): bool
+    {
+        foreach ($this->join as $loader) {
+            if ($loader->isDataDuplicationPossible()) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
     /**
      * @param AbstractNode $node
      */

src/Select/JoinableLoader.php

@@ -14,6 +14,7 @@
 use Cycle\ORM\Exception\LoaderException;
 use Cycle\ORM\ORMInterface;
 use Cycle\ORM\Parser\AbstractNode;
+use Cycle\ORM\Relation;
 use Cycle\ORM\Schema;
 use Cycle\ORM\Select\Traits\ColumnsTrait;
 use Cycle\ORM\Select\Traits\ScopeTrait;
@@ -233,6 +234,18 @@ public function configureQuery(SelectQuery $query, array $outerKeys = []): Selec
         return parent::configureQuery($query);
     }
 
+    public function isDataDuplicationPossible(): bool
+    {
+        $outerKey = $this->schema[Relation::OUTER_KEY];
+        $indexes = $this->orm->getIndexes($this->target);
+
+        if (!\in_array($outerKey, $indexes, true)) {
+            return true;
+        }
+
+        return parent::isDataDuplicationPossible();
+    }
+
     /**
      * @param SelectQuery $query
      *

src/Select/Loader/HasManyLoader.php

@@ -102,6 +102,11 @@ public function configureQuery(SelectQuery $query, array $outerKeys = []): Selec
         return parent::configureQuery($query);
     }
 
+    public function isDataDuplicationPossible(): bool
+    {
+        return true;
+    }
+
     /**
      * {@inheritdoc}
      */

src/Select/Loader/ManyToManyLoader.php

@@ -197,6 +197,11 @@ public function createNode(): AbstractNode
         return $node;
     }
 
+    public function isDataDuplicationPossible(): bool
+    {
+        return true;
+    }
+
     /**
      * @param AbstractNode $node
      */

src/Select/RootLoader.php

@@ -84,6 +84,20 @@ public function getPK(): string
         return $this->getAlias() . '.' . $this->fieldAlias($this->define(Schema::PRIMARY_KEY));
     }
 
+    /**
+     * Returns PK or DISTINCT(PK) if query has any join.
+     *
+     * @return string
+     */
+    public function getCountField(): string
+    {
+        if ($this->isDataDuplicationPossible()) {
+            return sprintf('DISTINCT(%s)', $this->getPK());
+        }
+
+        return '*';
+    }
+
     /**
      * Return base query associated with the loader.
      *

tests/ORM/PaginateTest.php

@@ -8,7 +8,6 @@
  */
 
 declare(strict_types=1);
-declare(strict_types=1);
 
 namespace Cycle\ORM\Tests;
 

tests/ORM/SelectorTest.php

@@ -16,7 +16,9 @@
 use Cycle\ORM\Schema;
 use Cycle\ORM\Select;
 use Cycle\ORM\Select\JoinableLoader;
+use Cycle\ORM\Select\RootLoader;
 use Cycle\ORM\Tests\Fixtures\Comment;
+use Cycle\ORM\Tests\Fixtures\Profile;
 use Cycle\ORM\Tests\Fixtures\User;
 use Cycle\ORM\Tests\Traits\TableTrait;
 
@@ -32,6 +34,7 @@ public function setUp(): void
             'id' => 'primary',
             'email' => 'string',
             'balance' => 'float',
+            'comment_id' => 'integer,nullable',
         ]);
 
         $this->getDatabase()->table('user')->insertMultiple(
@@ -42,6 +45,20 @@ public function setUp(): void
             ]
         );
 
+        $this->makeTable('profile', [
+            'id' => 'primary',
+            'user_id' => 'integer',
+            'image' => 'string',
+        ]);
+
+        $this->getDatabase()->table('profile')->insertMultiple(
+            ['user_id', 'image'],
+            [
+                [1, 'image.png'],
+                [2, 'second.png'],
+            ]
+        );
+
         $this->makeTable('comment', [
             'id' => 'primary',
             'user_id' => 'integer',
@@ -62,16 +79,40 @@ public function setUp(): void
             ]
         );
 
-        $this->orm = $this->withSchema(new Schema([
+        $this->orm = $this->withSchema(new Schema($this->getSchemaDefinition()));
+    }
+
+    private function getSchemaDefinition(): array
+    {
+        return [
             User::class => [
                 Schema::ROLE => 'user',
                 Schema::MAPPER => Mapper::class,
                 Schema::DATABASE => 'default',
                 Schema::TABLE => 'user',
                 Schema::PRIMARY_KEY => 'id',
-                Schema::COLUMNS => ['id', 'email', 'balance'],
+                Schema::COLUMNS => ['id', 'email', 'balance', 'comment_id'],
                 Schema::SCHEMA => [],
                 Schema::RELATIONS => [
+                    'profile' => [
+                        Relation::TYPE => Relation::HAS_ONE,
+                        Relation::TARGET => Profile::class,
+                        Relation::SCHEMA => [
+                            Relation::CASCADE => true,
+                            Relation::INNER_KEY => 'id',
+                            Relation::OUTER_KEY => 'user_id',
+                            Relation::NULLABLE => false,
+                        ],
+                    ],
+                    'lastComment' => [
+                        Relation::TYPE => Relation::REFERS_TO,
+                        Relation::TARGET => Comment::class,
+                        Relation::SCHEMA => [
+                            Relation::CASCADE => true,
+                            Relation::INNER_KEY => 'comment_id',
+                            Relation::OUTER_KEY => 'id',
+                        ],
+                    ],
                     'comments' => [
                         Relation::TYPE => Relation::HAS_MANY,
                         Relation::TARGET => Comment::class,
@@ -93,7 +134,29 @@ public function setUp(): void
                 Schema::SCHEMA => [],
                 Schema::RELATIONS => [],
             ],
-        ]));
+            Profile::class => [
+                Schema::ROLE => 'profile',
+                Schema::MAPPER => Mapper::class,
+                Schema::DATABASE => 'default',
+                Schema::TABLE => 'profile',
+                Schema::PRIMARY_KEY => 'id',
+                Schema::FIND_BY_KEYS => ['user_id'],
+                Schema::COLUMNS => ['id', 'user_id', 'image'],
+                Schema::SCHEMA => [],
+                Schema::RELATIONS => [
+                    'user' => [
+                        Relation::TYPE => Relation::BELONGS_TO,
+                        Relation::TARGET => User::class,
+                        Relation::SCHEMA => [
+                            Relation::CASCADE => true,
+                            Relation::INNER_KEY => 'user_id',
+                            Relation::OUTER_KEY => 'id',
+                            Relation::NULLABLE => false,
+                        ],
+                    ],
+                ],
+            ],
+        ];
     }
 
     public function testStableStatement(): void
@@ -133,4 +196,50 @@ public function testSelectCustomSQL(): void
             ],
         ], $result);
     }
+
+    public function testCount(): void
+    {
+        $this->assertSame(2, (new Select($this->orm, User::class))->count());
+        $this->assertSame(2, (new Select($this->orm, User::class))->with('profile')->count());
+        $this->assertSame(2, (new Select($this->orm, User::class))->with('lastComment', [
+            'method' => Select\AbstractLoader::LEFT_JOIN,
+        ])->count());
+        $this->assertSame(2, (new Select($this->orm, User::class))->with('comments')->count());
+
+        $schema = $this->getSchemaDefinition();
+        unset($schema[Profile::class][Schema::FIND_BY_KEYS]);
+        $this->orm = $this->withSchema(new Schema($schema));
+        $this->assertSame(2, (new Select($this->orm, User::class))->with('profile')->count());
+    }
+
+    public function testCountField(): void
+    {
+        $role = $this->orm->resolveRole(User::class);
+        $pk = \sprintf('%s.%s', $role, 'id');
+        $distinct = \sprintf('DISTINCT(%s)', $pk);
+
+        $this->assertSame('*', (new RootLoader($this->orm, $role))->getCountField());
+        $this->assertSame('*', $this->joinRelation(new RootLoader($this->orm, $role), 'profile')->getCountField());
+        $this->assertSame('*', $this->joinRelation(new RootLoader($this->orm, $role), 'lastComment', [
+            'method' => Select\AbstractLoader::LEFT_JOIN,
+        ])->getCountField());
+        $this->assertSame(
+            $distinct,
+            $this->joinRelation(new RootLoader($this->orm, $role), 'comments')->getCountField()
+        );
+
+        $schema = $this->getSchemaDefinition();
+        unset($schema[Profile::class][Schema::FIND_BY_KEYS]);
+        $this->orm = $this->withSchema(new Schema($schema));
+        $this->assertSame(
+            $distinct,
+            $this->joinRelation(new RootLoader($this->orm, $role), 'profile')->getCountField()
+        );
+    }
+
+    private function joinRelation(RootLoader $loader, string $relation, array $options = []): RootLoader
+    {
+        $loader->loadRelation($relation, $options, true);
+        return $loader;
+    }
 }