Merge pull request #698 from nextras/fix-lifting

Fix having lifting required by row aggregator

Author: hrach

src/Collection/Aggregations/Aggregator.php

@@ -34,4 +34,7 @@ public function aggregateExpression(
 		DbalExpressionResult $expression,
 		ExpressionContext $context,
 	): DbalExpressionResult;
+
+
+	public function isHavingClauseRequired(): bool;
 }

src/Collection/Aggregations/AnyAggregator.php

@@ -3,7 +3,6 @@
 namespace Nextras\Orm\Collection\Aggregations;
 
 
-use Nextras\Dbal\QueryBuilder\QueryBuilder;
 use Nextras\Orm\Collection\Expression\ExpressionContext;
 use Nextras\Orm\Collection\Functions\Result\DbalExpressionResult;
 use Nextras\Orm\Collection\Functions\Result\DbalTableJoin;
@@ -86,4 +85,10 @@ public function aggregateExpression(
 			havingArgs: [$join->toPrimaryKey],
 		);
 	}
+
+
+	public function isHavingClauseRequired(): bool
+	{
+		return false;
+	}
 }

src/Collection/Aggregations/CountAggregator.php

@@ -3,7 +3,6 @@
 namespace Nextras\Orm\Collection\Aggregations;
 
 
-use Nextras\Dbal\QueryBuilder\QueryBuilder;
 use Nextras\Orm\Collection\Expression\ExpressionContext;
 use Nextras\Orm\Collection\Functions\Result\DbalExpressionResult;
 use Nextras\Orm\Collection\Functions\Result\DbalTableJoin;
@@ -113,4 +112,10 @@ public function aggregateExpression(
 			);
 		}
 	}
+
+
+	public function isHavingClauseRequired(): bool
+	{
+		return true;
+	}
 }

src/Collection/Aggregations/NoneAggregator.php

@@ -3,7 +3,6 @@
 namespace Nextras\Orm\Collection\Aggregations;
 
 
-use Nextras\Dbal\QueryBuilder\QueryBuilder;
 use Nextras\Orm\Collection\Expression\ExpressionContext;
 use Nextras\Orm\Collection\Functions\Result\DbalExpressionResult;
 use Nextras\Orm\Collection\Functions\Result\DbalTableJoin;
@@ -80,4 +79,10 @@ public function aggregateExpression(
 			havingArgs: [$join->toPrimaryKey],
 		);
 	}
+
+
+	public function isHavingClauseRequired(): bool
+	{
+		return true;
+	}
 }

src/Collection/Aggregations/NumericAggregator.php

@@ -3,7 +3,6 @@
 namespace Nextras\Orm\Collection\Aggregations;
 
 
-use Nextras\Dbal\QueryBuilder\QueryBuilder;
 use Nextras\Orm\Collection\Expression\ExpressionContext;
 use Nextras\Orm\Collection\Functions\Result\DbalExpressionResult;
 
@@ -53,4 +52,10 @@ public function aggregateExpression(
 			havingArgs: $expression->args,
 		);
 	}
+
+
+	public function isHavingClauseRequired(): bool
+	{
+		return true;
+	}
 }

src/Collection/Functions/JunctionFunctionTrait.php

@@ -74,7 +74,7 @@ protected function processQueryBuilderExpressionWithModifier(
 		$expressions = [];
 		foreach ($normalized as $collectionFunctionArgs) {
 			$expressions[] = $expression = $helper->processExpression($builder, $collectionFunctionArgs, $aggregator);
-			if ($expression->havingExpression !== null) {
+			if ($expression->havingExpression !== null || ($expression->aggregator?->isHavingClauseRequired() ?? false)) {
 				$requiresHaving = true;
 			}
 		}

tests/cases/integration/Collection/collection.aggregation.phpt

@@ -8,6 +8,7 @@
 namespace NextrasTests\Orm\Integration\Collection;
 
 
+use Nextras\Orm\Collection\Aggregations\NoneAggregator;
 use Nextras\Orm\Collection\Functions\AvgAggregateFunction;
 use Nextras\Orm\Collection\Functions\CompareGreaterThanEqualsFunction;
 use Nextras\Orm\Collection\Functions\CompareGreaterThanFunction;
@@ -214,6 +215,31 @@ class CollectionAggregationTest extends DataTestCase
 		]);
 		Assert::same(3, $books->count()); // book #1, #2, #3
 	}
+
+
+	public function testRowAggregatorImposingLifting(): void
+	{
+		$books = $this->orm->books->findBy([
+			ICollection::OR,
+			['title' => 'Book 1'], // book #1
+			[ICollection::AND, new NoneAggregator(), 'tags->id' => 2], // book #3, #4
+		]);
+		Assert::same(3, $books->count());
+
+		$books = $this->orm->books->findBy([
+			ICollection::AND,
+			['title' => 'Book 1'], // book #1
+			[ICollection::AND, new NoneAggregator(), 'tags->id' => 2], // book #3, #4
+		]);
+		Assert::same(0, $books->count());
+
+		$books = $this->orm->books->findBy([
+			ICollection::AND,
+			['title' => 'Book 1'], // book #1
+			[ICollection::AND, new NoneAggregator(), 'tags->id' => 3], // book #1, #4
+		]);
+		Assert::same(1, $books->count());
+	}
 }
 
 

tests/sqls/NextrasTests/Orm/Integration/Collection/CollectionAggregationTest_testRowAggregatorImposingLifting.sql

@@ -0,0 +1,61 @@
+SELECT
+  "books".*
+FROM
+  "books" AS "books"
+  LEFT JOIN "books_x_tags" AS "books_x_tags_none" ON (
+    "books"."id" = "books_x_tags_none"."book_id"
+  )
+  LEFT JOIN "tags" AS "tags_none" ON (
+    (
+      "books_x_tags_none"."tag_id" = "tags_none"."id"
+    )
+    AND "tags_none"."id" = 2
+  )
+GROUP BY
+  "books"."title",
+  "books"."id"
+HAVING
+  ("books"."title" = 'Book 1')
+  OR (
+    COUNT("tags_none"."id") = 0
+  );
+
+SELECT
+  "books".*
+FROM
+  "books" AS "books"
+  LEFT JOIN "books_x_tags" AS "books_x_tags_none" ON (
+    "books"."id" = "books_x_tags_none"."book_id"
+  )
+  LEFT JOIN "tags" AS "tags_none" ON (
+    (
+      "books_x_tags_none"."tag_id" = "tags_none"."id"
+    )
+    AND "tags_none"."id" = 2
+  )
+WHERE
+  "books"."title" = 'Book 1'
+GROUP BY
+  "books"."id"
+HAVING
+  COUNT("tags_none"."id") = 0;
+
+SELECT
+  "books".*
+FROM
+  "books" AS "books"
+  LEFT JOIN "books_x_tags" AS "books_x_tags_none" ON (
+    "books"."id" = "books_x_tags_none"."book_id"
+  )
+  LEFT JOIN "tags" AS "tags_none" ON (
+    (
+      "books_x_tags_none"."tag_id" = "tags_none"."id"
+    )
+    AND "tags_none"."id" = 3
+  )
+WHERE
+  "books"."title" = 'Book 1'
+GROUP BY
+  "books"."id"
+HAVING
+  COUNT("tags_none"."id") = 0;