Skip to content

Commit 9e04377

Browse files
homersimpsonshomersimpsons
committed
findFromRawSql: fix COUNT with DISTINCT
1 parent 6c56e6a commit 9e04377

File tree

4 files changed

+77
-11
lines changed

4 files changed

+77
-11
lines changed

‎src/QueryFactory/FindObjectsFromRawSqlQueryFactory.php‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -144,14 +144,16 @@ private function processParsedUnionQuery(array $parsedSql, ?string $sqlCount): a
144144
private function processParsedSelectQuery(array $parsedSql, ?string $sqlCount): array
145145
{
146146
// 1: let's reformat the SELECT and construct our columns
147-
list($select, $columnDescriptors) = $this->formatSelect($parsedSql['SELECT']);
147+
list($select, $countSelect, $columnDescriptors) = $this->formatSelect($parsedSql['SELECT']);
148148
$generator = new PHPSQLCreator();
149149
$parsedSql['SELECT'] = $select;
150150
$processedSql = $generator->create($parsedSql);
151151

152152
// 2: let's compute the count query if needed
153153
if ($sqlCount === null) {
154-
$parsedSqlCount = $this->generateParsedSqlCount($parsedSql);
154+
$parsedCountSql = $parsedSql;
155+
$parsedCountSql['SELECT'] = $countSelect;
156+
$parsedSqlCount = $this->generateParsedSqlCount($parsedCountSql);
155157
$processedSqlCount = $generator->create($parsedSqlCount);
156158
} else {
157159
$processedSqlCount = $sqlCount;
@@ -173,40 +175,46 @@ private function formatSelect(array $baseSelect): array
173175

174176
$connection = $this->tdbmService->getConnection();
175177
$formattedSelect = [];
178+
$formattedCountSelect = [];
176179
$columnDescriptors = [];
177180
$fetchedTables = [];
178181

179182
foreach ($baseSelect as $entry) {
180183
if ($entry['expr_type'] !== 'colref') {
181184
$formattedSelect[] = $entry;
185+
$formattedCountSelect[] = $entry;
182186
continue;
183187
}
184188

185189
$noQuotes = $entry['no_quotes'];
186190
if ($noQuotes['delim'] !== '.' || count($noQuotes['parts']) !== 2) {
187191
$formattedSelect[] = $entry;
192+
$formattedCountSelect[] = $entry;
188193
continue;
189194
}
190195

191196
$tableName = $noQuotes['parts'][0];
192197
if (!in_array($tableName, $relatedTables)) {
193198
$formattedSelect[] = $entry;
199+
$formattedCountSelect[] = $entry;
194200
continue;
195201
}
196202

197203
$columnName = $noQuotes['parts'][1];
198204
if ($columnName !== '*') {
199205
$formattedSelect[] = $entry;
206+
$formattedCountSelect[] = $entry;
200207
continue;
201208
}
202209

203210
$table = $this->schema->getTable($tableName);
211+
$pkColumns = $table->getPrimaryKeyColumns();
204212
foreach ($table->getColumns() as $column) {
205213
$columnName = $column->getName();
206214
$alias = "{$tableName}____{$columnName}";
207-
$formattedSelect[] = [
215+
$astColumn = [
208216
'expr_type' => 'colref',
209-
'base_expr' => $connection->quoteIdentifier($tableName).'.'.$connection->quoteIdentifier($columnName),
217+
'base_expr' => $connection->quoteIdentifier($tableName) . '.' . $connection->quoteIdentifier($columnName),
210218
'no_quotes' => [
211219
'delim' => '.',
212220
'parts' => [
@@ -219,7 +227,10 @@ private function formatSelect(array $baseSelect): array
219227
'name' => $alias,
220228
]
221229
];
222-
230+
$formattedSelect[] = $astColumn;
231+
if (in_array($columnName, $pkColumns)) {
232+
$formattedCountSelect[] = $astColumn;
233+
}
223234
$columnDescriptors[$alias] = [
224235
'as' => $alias,
225236
'table' => $tableName,
@@ -241,7 +252,13 @@ private function formatSelect(array $baseSelect): array
241252
$formattedSelect[$i]['delim'] = ',';
242253
}
243254
}
244-
return [$formattedSelect, $columnDescriptors];
255+
256+
for ($i = 0; $i < count($formattedCountSelect) - 1; $i++) {
257+
if (!isset($formattedCountSelect[$i]['delim'])) {
258+
$formattedCountSelect[$i]['delim'] = ',';
259+
}
260+
}
261+
return [$formattedSelect, $formattedCountSelect, $columnDescriptors];
245262
}
246263

247264
/**

‎tests/Dao/TestAlbumDao.php‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
<?php
2+
declare(strict_types=1);
3+
4+
namespace TheCodingMachine\TDBM\Dao;
5+
6+
use TheCodingMachine\TDBM\Test\Dao\Bean\AlbumBean;
7+
use TheCodingMachine\TDBM\Test\Dao\Generated\AlbumBaseDao;
8+
9+
/**
10+
* The AlbumDao class will maintain the persistence of UserBean class into the users table.
11+
*/
12+
class TestAlbumDao extends AlbumBaseDao
13+
{
14+
/**
15+
* @return \TheCodingMachine\TDBM\ResultIterator|AlbumBean[]
16+
*/
17+
public function findAllFromRawSql()
18+
{
19+
return $this->findFromRawSql('SELECT DISTINCT albums.* FROM albums');
20+
}
21+
22+
/**
23+
* @return \TheCodingMachine\TDBM\ResultIterator|AlbumBean[]
24+
*/
25+
public function findAllFromRawSqlWithCount()
26+
{
27+
return $this->findFromRawSql(
28+
'SELECT DISTINCT albums.* FROM albums',
29+
[],
30+
'SELECT COUNT(DISTINCT albums.id) FROM albums'
31+
);
32+
}
33+
}

‎tests/Dao/TestPersonDao.php‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,14 @@
1616
*/
1717
class TestPersonDao extends PersonBaseDao
1818
{
19-
public function testFindFromRawSQLONInherited(): ResultIterator
19+
public function testFindFromRawSQLOnInherited(): ResultIterator
2020
{
2121
$sql = '
2222
SELECT DISTINCT person.*, contact.*, users.*
2323
FROM person JOIN contact ON person.id = contact.id
2424
JOIN users ON contact.id = users.id
25-
WHERE TRUE
2625
';
2726

2827
return $this->findFromRawSql($sql, []);
2928
}
30-
}
29+
}

‎tests/TDBMDaoGeneratorTest.php‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
use Ramsey\Uuid\Uuid;
3131
use ReflectionClass;
3232
use ReflectionMethod;
33+
use TheCodingMachine\TDBM\Dao\TestAlbumDao;
3334
use TheCodingMachine\TDBM\Dao\TestArticleDao;
3435
use TheCodingMachine\TDBM\Dao\TestCountryDao;
3536
use TheCodingMachine\TDBM\Dao\TestPersonDao;
@@ -43,6 +44,7 @@
4344
use TheCodingMachine\TDBM\Test\Dao\ArtistDao;
4445
use TheCodingMachine\TDBM\Test\Dao\BaseObjectDao;
4546
use TheCodingMachine\TDBM\Test\Dao\Bean\AccountBean;
47+
use TheCodingMachine\TDBM\Test\Dao\Bean\AlbumBean;
4648
use TheCodingMachine\TDBM\Test\Dao\Bean\AllNullableBean;
4749
use TheCodingMachine\TDBM\Test\Dao\Bean\AnimalBean;
4850
use TheCodingMachine\TDBM\Test\Dao\Bean\Article2Bean;
@@ -2180,12 +2182,27 @@ public function testFindByDateTime(): void
21802182
$this->assertTrue(true);
21812183
}
21822184

2185+
/**
2186+
* Bug: find from sql use a `COUNT(DISTINCT *)` which fails because of null values.
2187+
*/
2188+
public function testFindFromRawSqlCount(): void
2189+
{
2190+
$dao = new TestAlbumDao($this->tdbmService);
2191+
$albums = $dao->findAllFromRawSql();
2192+
2193+
$firstAlbum = $albums->first();
2194+
assert($firstAlbum instanceof AlbumBean);
2195+
$this->assertNull($firstAlbum->getNode()); // This null ensure reproducibility of the bug
2196+
$expectedCount = $dao->findAllFromRawSqlWithCount()->count();
2197+
$this->assertEquals($expectedCount, $albums->count());
2198+
}
2199+
21832200
public function testFindFromRawSQLOnInheritance(): void
21842201
{
21852202
$dao = new TestPersonDao($this->tdbmService);
2186-
$objects = $dao->testFindFromRawSQLONInherited();
2203+
$objects = $dao->testFindFromRawSQLOnInherited();
21872204

21882205
$this->assertNotNull($objects->first());
2189-
$this->assertNotEquals(0, $objects->count());
2206+
$this->assertEquals(6, $objects->count());
21902207
}
21912208
}

0 commit comments

Comments
 (0)