Skip to content

Commit

Permalink
Merge pull request #664 from nextras/having-same-named-cols-groupby
Browse files Browse the repository at this point in the history
Add workaround for same-named columns in GROUP BY for MySQL
  • Loading branch information
hrach committed Apr 10, 2024
2 parents 3b71eb8 + 974682d commit 54ac7f8
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 0 deletions.
33 changes: 33 additions & 0 deletions src/Collection/DbalCollection.php
Expand Up @@ -5,6 +5,8 @@

use Iterator;
use Nextras\Dbal\IConnection;
use Nextras\Dbal\Platforms\Data\Fqn;
use Nextras\Dbal\Platforms\MySqlPlatform;
use Nextras\Dbal\QueryBuilder\QueryBuilder;
use Nextras\Orm\Collection\Expression\ExpressionContext;
use Nextras\Orm\Collection\Functions\Result\DbalExpressionResult;
Expand Down Expand Up @@ -319,6 +321,9 @@ public function getQueryBuilder(): QueryBuilder
} else {
$this->queryBuilder->andWhere($expression->expression, ...$expression->args);
}
if ($this->mapper->getDatabasePlatform()->getName() === MySqlPlatform::NAME) {
$this->applyGroupByWithSameNamedColumnsWorkaround($this->queryBuilder, $groupBy);
}
$this->filtering = [];
}

Expand Down Expand Up @@ -400,4 +405,32 @@ protected function getHelper(): DbalQueryBuilderHelper

return $this->helper;
}


/**
* Apply workaround for MySQL that is not able to properly resolve columns when there are more same-named
* columns in the GROUP BY clause, even though they are properly referenced to their tables. Orm workarounds
* this by adding them to the SELECT clause and renames them not to conflict anywhere.
*
* @param list<Fqn> $groupBy
*/
private function applyGroupByWithSameNamedColumnsWorkaround(QueryBuilder $queryBuilder, array $groupBy): void
{
$map = [];
foreach ($groupBy as $fqn) {
if (!isset($map[$fqn->name])) {
$map[$fqn->name] = [$fqn];
} else {
$map[$fqn->name][] = $fqn;
}
}
$i = 0;
foreach ($map as $fqns) {
if (count($fqns) > 1) {
foreach ($fqns as $fqn) {
$queryBuilder->addSelect("%column AS __nextras_fix_" . $i++, $fqn); // @phpstan-ignore-line
}
}
}
}
}
36 changes: 36 additions & 0 deletions tests/cases/integration/Collection/collection.having.phpt
@@ -0,0 +1,36 @@
<?php declare(strict_types = 1);

/**
* @testCase
* @dataProvider ../../../databases.ini
*/

namespace NextrasTests\Orm\Integration\Collection;


use Nextras\Orm\Collection\ICollection;
use NextrasTests\Orm\DataTestCase;
use Tester\Assert;


require_once __DIR__ . '/../../../bootstrap.php';


class CollectionHavingTest extends DataTestCase
{
public function testHavingWithSameNamedColumnsInGroupBy(): void
{
// this is a test especially for MySQL and Orm's workaround
$books = $this->orm->books->findBy([
ICollection::OR,
'tags->id' => 1,
'author->name' => 'Writer 1',
'publisher->name' => 'Nextras publisher A',
]);
Assert::same($books->count(), 3);
}
}


$test = new CollectionHavingTest();
$test->run();
@@ -0,0 +1 @@
SELECT "books".* FROM "books" AS "books" LEFT JOIN "books_x_tags" AS "books_x_tags" ON ("books"."id" = "books_x_tags"."book_id") LEFT JOIN "tags" AS "tags_any" ON (("books_x_tags"."tag_id" = "tags_any"."id") AND "tags_any"."id" = 1) LEFT JOIN "public"."authors" AS "author" ON ("books"."author_id" = "author"."id") LEFT JOIN "publishers" AS "publisher" ON ("books"."publisher_id" = "publisher"."publisher_id") GROUP BY "books"."id", "author"."name", "publisher"."name" HAVING ((COUNT("tags_any"."id") > 0) OR ("author"."name" = 'Writer 1') OR ("publisher"."name" = 'Nextras publisher A'));

0 comments on commit 54ac7f8

Please sign in to comment.