findFromRawSql: fix COUNT with DISTINCT

thecodingmachine · Apr 24, 2020 · 9e04377 · 9e04377
1 parent 6c56e6a
commit 9e04377
Show file tree

Hide file tree

Showing 4 changed files with 77 additions and 11 deletions.
diff --git a/src/QueryFactory/FindObjectsFromRawSqlQueryFactory.php b/src/QueryFactory/FindObjectsFromRawSqlQueryFactory.php
@@ -144,14 +144,16 @@ private function processParsedUnionQuery(array $parsedSql, ?string $sqlCount): a
     private function processParsedSelectQuery(array $parsedSql, ?string $sqlCount): array
     {
         // 1: let's reformat the SELECT and construct our columns
-        list($select, $columnDescriptors) = $this->formatSelect($parsedSql['SELECT']);
+        list($select, $countSelect, $columnDescriptors) = $this->formatSelect($parsedSql['SELECT']);
         $generator = new PHPSQLCreator();
         $parsedSql['SELECT'] = $select;
         $processedSql = $generator->create($parsedSql);
 
         // 2: let's compute the count query if needed
         if ($sqlCount === null) {
-            $parsedSqlCount = $this->generateParsedSqlCount($parsedSql);
+            $parsedCountSql = $parsedSql;
+            $parsedCountSql['SELECT'] = $countSelect;
+            $parsedSqlCount = $this->generateParsedSqlCount($parsedCountSql);
             $processedSqlCount = $generator->create($parsedSqlCount);
         } else {
             $processedSqlCount = $sqlCount;
@@ -173,40 +175,46 @@ private function formatSelect(array $baseSelect): array
 
         $connection = $this->tdbmService->getConnection();
         $formattedSelect = [];
+        $formattedCountSelect = [];
         $columnDescriptors = [];
         $fetchedTables = [];
 
         foreach ($baseSelect as $entry) {
             if ($entry['expr_type'] !== 'colref') {
                 $formattedSelect[] = $entry;
+                $formattedCountSelect[] = $entry;
                 continue;
             }
 
             $noQuotes = $entry['no_quotes'];
             if ($noQuotes['delim'] !== '.' || count($noQuotes['parts']) !== 2) {
                 $formattedSelect[] = $entry;
+                $formattedCountSelect[] = $entry;
                 continue;
             }
 
             $tableName = $noQuotes['parts'][0];
             if (!in_array($tableName, $relatedTables)) {
                 $formattedSelect[] = $entry;
+                $formattedCountSelect[] = $entry;
                 continue;
             }
 
             $columnName = $noQuotes['parts'][1];
             if ($columnName !== '*') {
                 $formattedSelect[] = $entry;
+                $formattedCountSelect[] = $entry;
                 continue;
             }
 
             $table = $this->schema->getTable($tableName);
+            $pkColumns = $table->getPrimaryKeyColumns();
             foreach ($table->getColumns() as $column) {
                 $columnName = $column->getName();
                 $alias = "{$tableName}____{$columnName}";
-                $formattedSelect[] = [
+                $astColumn = [
                     'expr_type' => 'colref',
-                    'base_expr' => $connection->quoteIdentifier($tableName).'.'.$connection->quoteIdentifier($columnName),
+                    'base_expr' => $connection->quoteIdentifier($tableName) . '.' . $connection->quoteIdentifier($columnName),
                     'no_quotes' => [
                         'delim' => '.',
                         'parts' => [
@@ -219,7 +227,10 @@ private function formatSelect(array $baseSelect): array
                         'name' => $alias,
                     ]
                 ];
-
+                $formattedSelect[] = $astColumn;
+                if (in_array($columnName, $pkColumns)) {
+                    $formattedCountSelect[] = $astColumn;
+                }
                 $columnDescriptors[$alias] = [
                     'as' => $alias,
                     'table' => $tableName,
@@ -241,7 +252,13 @@ private function formatSelect(array $baseSelect): array
                 $formattedSelect[$i]['delim'] = ',';
             }
         }
-        return [$formattedSelect, $columnDescriptors];
+
+        for ($i = 0; $i < count($formattedCountSelect) - 1; $i++) {
+            if (!isset($formattedCountSelect[$i]['delim'])) {
+                $formattedCountSelect[$i]['delim'] = ',';
+            }
+        }
+        return [$formattedSelect, $formattedCountSelect, $columnDescriptors];
     }
 
     /**

diff --git a/tests/Dao/TestAlbumDao.php b/tests/Dao/TestAlbumDao.php
@@ -0,0 +1,33 @@
+<?php
+declare(strict_types=1);
+
+namespace TheCodingMachine\TDBM\Dao;
+
+use TheCodingMachine\TDBM\Test\Dao\Bean\AlbumBean;
+use TheCodingMachine\TDBM\Test\Dao\Generated\AlbumBaseDao;
+
+/**
+ * The AlbumDao class will maintain the persistence of UserBean class into the users table.
+ */
+class TestAlbumDao extends AlbumBaseDao
+{
+    /**
+     * @return \TheCodingMachine\TDBM\ResultIterator|AlbumBean[]
+     */
+    public function findAllFromRawSql()
+    {
+        return $this->findFromRawSql('SELECT DISTINCT albums.* FROM albums');
+    }
+
+    /**
+     * @return \TheCodingMachine\TDBM\ResultIterator|AlbumBean[]
+     */
+    public function findAllFromRawSqlWithCount()
+    {
+        return $this->findFromRawSql(
+            'SELECT DISTINCT albums.* FROM albums',
+            [],
+            'SELECT COUNT(DISTINCT albums.id) FROM albums'
+        );
+    }
+}
diff --git a/tests/Dao/TestPersonDao.php b/tests/Dao/TestPersonDao.php
@@ -16,15 +16,14 @@
  */
 class TestPersonDao extends PersonBaseDao
 {
-    public function testFindFromRawSQLONInherited(): ResultIterator
+    public function testFindFromRawSQLOnInherited(): ResultIterator
     {
         $sql = '
             SELECT DISTINCT  person.*, contact.*, users.*
             FROM person JOIN contact ON person.id = contact.id
              JOIN users ON contact.id = users.id
-            WHERE TRUE
         ';
 
         return $this->findFromRawSql($sql, []);
     }
-}
+}
diff --git a/tests/TDBMDaoGeneratorTest.php b/tests/TDBMDaoGeneratorTest.php
@@ -30,6 +30,7 @@
 use Ramsey\Uuid\Uuid;
 use ReflectionClass;
 use ReflectionMethod;
+use TheCodingMachine\TDBM\Dao\TestAlbumDao;
 use TheCodingMachine\TDBM\Dao\TestArticleDao;
 use TheCodingMachine\TDBM\Dao\TestCountryDao;
 use TheCodingMachine\TDBM\Dao\TestPersonDao;
@@ -43,6 +44,7 @@
 use TheCodingMachine\TDBM\Test\Dao\ArtistDao;
 use TheCodingMachine\TDBM\Test\Dao\BaseObjectDao;
 use TheCodingMachine\TDBM\Test\Dao\Bean\AccountBean;
+use TheCodingMachine\TDBM\Test\Dao\Bean\AlbumBean;
 use TheCodingMachine\TDBM\Test\Dao\Bean\AllNullableBean;
 use TheCodingMachine\TDBM\Test\Dao\Bean\AnimalBean;
 use TheCodingMachine\TDBM\Test\Dao\Bean\Article2Bean;
@@ -2180,12 +2182,27 @@ public function testFindByDateTime(): void
         $this->assertTrue(true);
     }
 
+    /**
+     * Bug: find from sql use a `COUNT(DISTINCT *)` which fails because of null values.
+     */
+    public function testFindFromRawSqlCount(): void
+    {
+        $dao = new TestAlbumDao($this->tdbmService);
+        $albums = $dao->findAllFromRawSql();
+
+        $firstAlbum = $albums->first();
+        assert($firstAlbum instanceof AlbumBean);
+        $this->assertNull($firstAlbum->getNode()); // This null ensure reproducibility of the bug
+        $expectedCount = $dao->findAllFromRawSqlWithCount()->count();
+        $this->assertEquals($expectedCount, $albums->count());
+    }
+
     public function testFindFromRawSQLOnInheritance(): void
     {
         $dao = new TestPersonDao($this->tdbmService);
-        $objects = $dao->testFindFromRawSQLONInherited();
+        $objects = $dao->testFindFromRawSQLOnInherited();
 
         $this->assertNotNull($objects->first());
-        $this->assertNotEquals(0, $objects->count());
+        $this->assertEquals(6, $objects->count());
     }
 }