relationships: prepare columns to be later optionally included in gro…

…uping [closes #548]
nextras · Mar 6, 2022 · 4c4310f · 4c4310f
1 parent 1d3b57d
commit 4c4310f
Show file tree

Hide file tree

Showing 7 changed files with 24 additions and 11 deletions.
diff --git a/src/Collection/Aggregations/CountAggregator.php b/src/Collection/Aggregations/CountAggregator.php
@@ -76,13 +76,12 @@ public function aggregateExpression(
 		);
 
 		$primaryKey = $join->conventions->getStoragePrimaryKey()[0];
-		$groupBy = $expression->groupBy;
 
 		return new DbalExpressionResult(
 			'COUNT(%table.%column) >= %i AND COUNT(%table.%column) <= %i',
 			[$join->toAlias, $primaryKey, $this->atLeast, $join->toAlias, $primaryKey, $this->atMost],
 			$joins,
-			$groupBy,
+			$expression->groupBy,
 			null,
 			true,
 			null,

diff --git a/src/Collection/DbalCollection.php b/src/Collection/DbalCollection.php
@@ -332,8 +332,8 @@ public function getQueryBuilder(): QueryBuilder
 				null
 			);
 			$joins = $expression->joins;
-			$groupBy = $expression->groupBy;
 			if ($expression->isHavingClause) {
+				$groupBy = $expression->groupBy;
 				$this->queryBuilder->andHaving($expression->expression, ...$expression->args);
 			} else {
 				$this->queryBuilder->andWhere($expression->expression, ...$expression->args);
@@ -343,7 +343,9 @@ public function getQueryBuilder(): QueryBuilder
 
 		foreach ($this->ordering as [$expression, $direction]) {
 			$joins = array_merge($joins, $expression->joins);
-			$groupBy = array_merge($groupBy, $expression->groupBy);
+			if ($expression->isHavingClause) {
+				$groupBy = array_merge($groupBy, $expression->groupBy);
+			}
 			$orderingExpression = $helper->processOrderDirection($expression, $direction);
 			$this->queryBuilder->addOrderBy('%ex', $orderingExpression);
 		}

diff --git a/src/Collection/Helpers/DbalExpressionResult.php b/src/Collection/Helpers/DbalExpressionResult.php
@@ -38,6 +38,7 @@ class DbalExpressionResult
 
 	/**
 	 * List of arguments possible to pass to %ex modifier.
+	 * Those grouping expressions are applied iff the $isHavingClause is true.
 	 * @var array<array<mixed>>
 	 */
 	public $groupBy;

diff --git a/src/Collection/Helpers/DbalQueryBuilderHelper.php b/src/Collection/Helpers/DbalQueryBuilderHelper.php
@@ -322,12 +322,6 @@ private function processTokens(
 			}
 		}
 
-		if ($makeDistinct) {
-			$groupBy = $this->makeDistinct($builder, $this->mapper);
-		} else {
-			$groupBy = [];
-		}
-
 		$propertyMetadata = $currentEntityMetadata->getProperty($lastToken);
 		if ($propertyMetadata->wrapper === EmbeddableContainer::class) {
 			$propertyExpression = implode('->', array_merge($tokens, [$lastToken]));
@@ -342,6 +336,12 @@ private function processTokens(
 			$propertyPrefixTokens
 		);
 
+		if ($makeDistinct) {
+			$groupBy = $this->makeDistinct($builder, $this->mapper);
+		} else {
+			$groupBy = [['%column', $column]];
+		}
+
 		return new DbalExpressionResult(
 			'%column',
 			[$column],

diff --git a/tests/cases/integration/Relationships/relationships.manyHasOne.phpt b/tests/cases/integration/Relationships/relationships.manyHasOne.phpt
@@ -143,6 +143,16 @@ class RelationshipManyHasOneTest extends DataTestCase
 		}
 		Assert::equal(['Nextras publisher A', 'Nextras publisher C'], $publishers);
 	}
+
+
+	public function testProperAggregation(): void
+	{
+		$books = $this->orm->books->findBy([
+			'tags->id' => 1,
+			'publisher->name' => 'Nextras publisher A',
+		]);
+		Assert::same($books->count(), 1);
+	}
 }
 
 

diff --git a/...sTests/Orm/Integration/Relationships/RelationshipManyHasOneTest_testProperAggregation.sql b/...sTests/Orm/Integration/Relationships/RelationshipManyHasOneTest_testProperAggregation.sql
@@ -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 "publishers" AS "publisher" ON ("books"."publisher_id" = "publisher"."publisher_id") GROUP BY "books"."id", "publisher"."name" HAVING ((COUNT("tags_any"."id") > 0) AND ("publisher"."name" = 'Nextras publisher A'));
diff --git a/...tionships/RelationshipOneHasManyTest_testCountStoredOnOneHasManyRelationshipCondition.sql b/...tionships/RelationshipOneHasManyTest_testCountStoredOnOneHasManyRelationshipCondition.sql
@@ -1,3 +1,3 @@
 SELECT "publishers".* FROM "publishers" AS "publishers" WHERE (("publishers"."publisher_id" = 1));
 SELECT "publisher_id", COUNT(DISTINCT "count") as "count" FROM (SELECT "books".*, "books"."id" AS "count" 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) WHERE "books"."publisher_id" IN (1) GROUP BY "books"."id" HAVING ((COUNT("tags_any"."id") > 0))) AS "temp" GROUP BY "publisher_id";
-SELECT "publisher_id", COUNT(DISTINCT "count") as "count" FROM (SELECT "books".*, "books"."id" AS "count" 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) WHERE "books"."publisher_id" IN (1) GROUP BY "books"."id" HAVING (("books"."title" = 'Book 1') OR (COUNT("tags_any"."id") > 0))) AS "temp" GROUP BY "publisher_id";
+SELECT "publisher_id", COUNT(DISTINCT "count") as "count" FROM (SELECT "books".*, "books"."id" AS "count" 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) WHERE "books"."publisher_id" IN (1) GROUP BY "books"."title", "books"."id" HAVING (("books"."title" = 'Book 1') OR (COUNT("tags_any"."id") > 0))) AS "temp" GROUP BY "publisher_id";