From 9bb40ad169903ea4461f2759de4f525cef7e81f5 Mon Sep 17 00:00:00 2001
From: Jan Skrasek <hrach.cz@gmail.com>
Date: Sat, 2 Nov 2024 22:44:07 +0100
Subject: [PATCH] fix for having lifting required by row aggregator

[closes #697]
---
 src/Collection/Aggregations/Aggregator.php    |  3 +
 src/Collection/Aggregations/AnyAggregator.php |  7 ++-
 .../Aggregations/CountAggregator.php          |  7 ++-
 .../Aggregations/NoneAggregator.php           |  7 ++-
 .../Aggregations/NumericAggregator.php        |  7 ++-
 .../Functions/JunctionFunctionTrait.php       |  2 +-
 .../Collection/collection.aggregation.phpt    | 26 ++++++++
 ...nTest_testRowAggregatorImposingLifting.sql | 61 +++++++++++++++++++
 8 files changed, 115 insertions(+), 5 deletions(-)
 create mode 100644 tests/sqls/NextrasTests/Orm/Integration/Collection/CollectionAggregationTest_testRowAggregatorImposingLifting.sql

diff --git a/src/Collection/Aggregations/Aggregator.php b/src/Collection/Aggregations/Aggregator.php
index 02824a0b..15afe85b 100644
--- a/src/Collection/Aggregations/Aggregator.php
+++ b/src/Collection/Aggregations/Aggregator.php
@@ -34,4 +34,7 @@ public function aggregateExpression(
 		DbalExpressionResult $expression,
 		ExpressionContext $context,
 	): DbalExpressionResult;
+
+
+	public function isHavingClauseRequired(): bool;
 }
diff --git a/src/Collection/Aggregations/AnyAggregator.php b/src/Collection/Aggregations/AnyAggregator.php
index 32a4c6f2..a1cbcd20 100644
--- a/src/Collection/Aggregations/AnyAggregator.php
+++ b/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;
+	}
 }
diff --git a/src/Collection/Aggregations/CountAggregator.php b/src/Collection/Aggregations/CountAggregator.php
index 7e2797d9..ee12fa6f 100644
--- a/src/Collection/Aggregations/CountAggregator.php
+++ b/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;
+	}
 }
diff --git a/src/Collection/Aggregations/NoneAggregator.php b/src/Collection/Aggregations/NoneAggregator.php
index 85fde472..c00faf6e 100644
--- a/src/Collection/Aggregations/NoneAggregator.php
+++ b/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;
+	}
 }
diff --git a/src/Collection/Aggregations/NumericAggregator.php b/src/Collection/Aggregations/NumericAggregator.php
index 1b69568e..6559a880 100644
--- a/src/Collection/Aggregations/NumericAggregator.php
+++ b/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;
+	}
 }
diff --git a/src/Collection/Functions/JunctionFunctionTrait.php b/src/Collection/Functions/JunctionFunctionTrait.php
index e9e37a06..5bf3e7af 100644
--- a/src/Collection/Functions/JunctionFunctionTrait.php
+++ b/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;
 			}
 		}
diff --git a/tests/cases/integration/Collection/collection.aggregation.phpt b/tests/cases/integration/Collection/collection.aggregation.phpt
index 3a3720c2..766cfd58 100644
--- a/tests/cases/integration/Collection/collection.aggregation.phpt
+++ b/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());
+	}
 }
 
 
diff --git a/tests/sqls/NextrasTests/Orm/Integration/Collection/CollectionAggregationTest_testRowAggregatorImposingLifting.sql b/tests/sqls/NextrasTests/Orm/Integration/Collection/CollectionAggregationTest_testRowAggregatorImposingLifting.sql
new file mode 100644
index 00000000..5d986419
--- /dev/null
+++ b/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;