Aggregate functions affect nullability only when the query is not grouped

Author: arnaud-lb

src/Type/Doctrine/Query/QueryResultTypeWalker.php

@@ -97,7 +97,10 @@ class QueryResultTypeWalker extends SqlWalker
 	private $descriptorRegistry;
 
 	/** @var bool */
-	private $isAggregated;
+	private $hasAggregateFunction;
+
+	/** @var bool */
+	private $hasGroupByClause;
 
 	/**
 	 * @param Query<mixed> $query
@@ -125,7 +128,8 @@ public function __construct($query, $parserResult, array $queryComponents)
 		$this->em = $query->getEntityManager();
 		$this->queryComponents = $queryComponents;
 		$this->nullableQueryComponents = [];
-		$this->isAggregated = false;
+		$this->hasAggregateFunction = false;
+		$this->hasGroupByClause = false;
 
 		// The object is instantiated by Doctrine\ORM\Query\Parser, so receiving
 		// dependencies through the constructor is not an option. Instead, we
@@ -166,7 +170,8 @@ public function __construct($query, $parserResult, array $queryComponents)
 	public function walkSelectStatement(AST\SelectStatement $AST)
 	{
 		$this->typeBuilder->setSelectQuery();
-		$this->isAggregated = $this->isAggregated($AST);
+		$this->hasAggregateFunction = $this->hasAggregateFunction($AST);
+		$this->hasGroupByClause = $AST->groupByClause !== null;
 
 		$this->walkFromClause($AST->fromClause);
 
@@ -231,7 +236,9 @@ public function walkPathExpression($pathExpr)
 
 				assert(is_string($typeName));
 
-				$nullable = $this->isQueryComponentNullable($dqlAlias) || $class->isNullable($fieldName) || $this->isAggregated;
+				$nullable = $this->isQueryComponentNullable($dqlAlias)
+					|| $class->isNullable($fieldName)
+					|| $this->hasAggregateWithoutGroupBy();
 
 				$fieldType = $this->resolveDatabaseInternalType($typeName, $nullable);
 
@@ -267,7 +274,8 @@ public function walkPathExpression($pathExpr)
 
 				assert(is_string($typeName));
 
-				$nullable = (bool) ($joinColumn['nullable'] ?? true) || $this->isAggregated;
+				$nullable = (bool) ($joinColumn['nullable'] ?? true)
+					|| $this->hasAggregateWithoutGroupBy();
 
 				$fieldType = $this->resolveDatabaseInternalType($typeName, $nullable);
 
@@ -699,7 +707,7 @@ public function walkSelectExpression($selectExpression)
 
 			$type = new ObjectType($class->name);
 
-			if ($this->isQueryComponentNullable($dqlAlias) || $this->isAggregated) {
+			if ($this->isQueryComponentNullable($dqlAlias) || $this->hasAggregateWithoutGroupBy()) {
 				$type = TypeCombinator::addNull($type);
 			}
 
@@ -725,7 +733,9 @@ public function walkSelectExpression($selectExpression)
 
 			assert(is_string($typeName));
 
-			$nullable = $this->isQueryComponentNullable($dqlAlias) || $class->isNullable($fieldName) || $this->isAggregated;
+			$nullable = $this->isQueryComponentNullable($dqlAlias)
+				|| $class->isNullable($fieldName)
+				|| $this->hasAggregateWithoutGroupBy();
 
 			$type = $this->resolveDoctrineType($typeName, $nullable);
 
@@ -1288,12 +1298,22 @@ private function toNumericOrNull(Type $type): Type
 		});
 	}
 
-	private function isAggregated(AST\SelectStatement $AST): bool
+	/**
+	 * Returns whether the query has aggregate function and no group by clause
+	 *
+	 * Queries with aggregate functions and no group by clause always have
+	 * exactly 1 group. This implies that they return exactly 1 row, and that
+	 * all column can have a null value.
+	 *
+	 * c.f. SQL92, section 7.9, General Rules
+	 */
+	private function hasAggregateWithoutGroupBy(): bool
 	{
-		if ($AST->groupByClause !== null) {
-			return true;
-		}
+		return $this->hasAggregateFunction && !$this->hasGroupByClause;
+	}
 
+	private function hasAggregateFunction(AST\SelectStatement $AST): bool
+	{
 		foreach ($AST->selectClause->selectExpressions as $selectExpression) {
 			if (!$selectExpression instanceof AST\SelectExpression) {
 				continue;

tests/Type/Doctrine/Query/QueryResultTypeWalkerTest.php

@@ -538,19 +538,19 @@ public function getTestData(): array
 				$this->constantArray([
 					[
 						new ConstantStringType('intColumn'),
-						TypeCombinator::addNull(new IntegerType()),
+						new IntegerType(),
 					],
 					[
 						new ConstantStringType('max'),
 						TypeCombinator::addNull($this->intStringified()),
 					],
 					[
 						new ConstantStringType('arithmetic'),
-						TypeCombinator::addNull($this->intStringified()),
+						$this->intStringified(),
 					],
 					[
 						new ConstantStringType('coalesce'),
-						TypeCombinator::addNull($this->intStringified()),
+						$this->intStringified(),
 					],
 					[
 						new ConstantStringType('count'),