remove NULL from simple cases of SUM GROUP BY etc

schlndh · Jun 22, 2024 · 3080e3d · 3080e3d
1 parent ca4c06d
commit 3080e3d
Show file tree

Hide file tree

Showing 23 changed files with 189 additions and 23 deletions.
diff --git a/src/Analyser/AnalyserState.php b/src/Analyser/AnalyserState.php
@@ -387,7 +387,7 @@ private function analyseSingleSelectQuery(SimpleSelectQuery $select): array
 				case SelectExprTypeEnum::REGULAR_EXPR:
 					assert($selectExpr instanceof RegularExpr);
 					$expr = $this->removeUnaryPlusPrefix($selectExpr->expr);
-					$resolvedExpr = $this->resolveExprType($expr);
+					$resolvedExpr = $this->resolveExprType($expr, null, $select->groupBy !== null);
 					$resolvedField = new QueryResultField(
 						$selectExpr->alias ?? $this->getDefaultFieldNameForExpr($expr),
 						$resolvedExpr,
@@ -781,8 +781,12 @@ private function analyseTruncateQuery(TruncateQuery $query): void
 	}
 
 	/** @throws AnalyserException */
-	private function resolveExprType(Expr\Expr $expr, ?AnalyserConditionTypeEnum $condition = null): ExprTypeResult
-	{
+	private function resolveExprType(
+		Expr\Expr $expr,
+		?AnalyserConditionTypeEnum $condition = null,
+		// false doesn't mean that the result set is empty, it may be empty.
+		bool $isNonEmptyAggResultSet = false,
+	): ExprTypeResult {
 		// TODO: handle all expression types
 		switch ($expr::getExprType()) {
 			case Expr\ExprTypeEnum::COLUMN:
@@ -851,7 +855,11 @@ private function resolveExprType(Expr\Expr $expr, ?AnalyserConditionTypeEnum $co
 					AnalyserConditionTypeEnum::NULL => AnalyserConditionTypeEnum::NULL,
 					default => AnalyserConditionTypeEnum::NOT_NULL,
 				};
-				$timeQuantityResult = $this->resolveExprType($expr->timeQuantity, $innerCondition);
+				$timeQuantityResult = $this->resolveExprType(
+					$expr->timeQuantity,
+					$innerCondition,
+					$isNonEmptyAggResultSet,
+				);
 
 				return new ExprTypeResult(
 					new Schema\DbType\DateTimeType(),
@@ -880,7 +888,11 @@ private function resolveExprType(Expr\Expr $expr, ?AnalyserConditionTypeEnum $co
 					$isTruthinessUncertain = true;
 				}
 
-				$resolvedInnerExpr = $this->resolveExprType($expr->expression, $innerCondition);
+				$resolvedInnerExpr = $this->resolveExprType(
+					$expr->expression,
+					$innerCondition,
+					$isNonEmptyAggResultSet,
+				);
 
 				$type = match ($expr->operation) {
 					Expr\UnaryOpTypeEnum::PLUS => $resolvedInnerExpr->type,
@@ -990,8 +1002,8 @@ private function resolveExprType(Expr\Expr $expr, ?AnalyserConditionTypeEnum $co
 					$kbCombinineWithAnd = $innerConditionLeft === AnalyserConditionTypeEnum::NOT_NULL;
 				}
 
-				$leftResult = $this->resolveExprType($expr->left, $innerConditionLeft);
-				$rightResult = $this->resolveExprType($expr->right, $innerConditionRight);
+				$leftResult = $this->resolveExprType($expr->left, $innerConditionLeft, $isNonEmptyAggResultSet);
+				$rightResult = $this->resolveExprType($expr->right, $innerConditionRight, $isNonEmptyAggResultSet);
 				$type = null;
 
 				if (
@@ -1116,6 +1128,7 @@ private function resolveExprType(Expr\Expr $expr, ?AnalyserConditionTypeEnum $co
 			case Expr\ExprTypeEnum::SUBQUERY:
 				assert($expr instanceof Expr\Subquery);
 				// TODO: handle $condition
+				// TODO: handle $isNonEmptyAggResultSet
 				$subqueryAnalyser = $this->getSubqueryAnalyser($expr->query);
 				$result = $subqueryAnalyser->analyse();
 				$canBeEmpty = ($result->rowCountRange->min ?? 0) === 0;
@@ -1188,7 +1201,10 @@ private function resolveExprType(Expr\Expr $expr, ?AnalyserConditionTypeEnum $co
 				assert($expr instanceof Expr\Between);
 				// TODO: handle $condition
 				$isNullable = array_reduce(
-					array_map($this->resolveExprType(...), [$expr->expression, $expr->min, $expr->max]),
+					array_map(
+						fn (Expr\Expr $e) => $this->resolveExprType($e, null, $isNonEmptyAggResultSet),
+						[$expr->expression, $expr->min, $expr->max],
+					),
 					static fn (bool $isNullable, ExprTypeResult $f) => $isNullable || $f->isNullable,
 					false,
 				);
@@ -1216,7 +1232,7 @@ private function resolveExprType(Expr\Expr $expr, ?AnalyserConditionTypeEnum $co
 				};
 				$kbCombinineWithAnd = $innerCondition === AnalyserConditionTypeEnum::NOT_NULL;
 				$innerFields = array_map(
-					fn (Expr\Expr $e) => $this->resolveExprType($e, $innerCondition),
+					fn (Expr\Expr $e) => $this->resolveExprType($e, $innerCondition, $isNonEmptyAggResultSet),
 					$expr->expressions,
 				);
 				$innerTypes = array_map(static fn (ExprTypeResult $f) => $f->type, $innerFields);
@@ -1261,8 +1277,8 @@ private function resolveExprType(Expr\Expr $expr, ?AnalyserConditionTypeEnum $co
 					AnalyserConditionTypeEnum::NULL => [AnalyserConditionTypeEnum::NULL, null],
 					null => [null, null],
 				};
-				$leftResult = $this->resolveExprType($expr->left, $innerConditionLeft);
-				$rightResult = $this->resolveExprType($expr->right, $innerConditionRight);
+				$leftResult = $this->resolveExprType($expr->left, $innerConditionLeft, $isNonEmptyAggResultSet);
+				$rightResult = $this->resolveExprType($expr->right, $innerConditionRight, $isNonEmptyAggResultSet);
 				$rightType = $rightResult->type;
 
 				// $rightType may not be a tuple if it's a subquery (e.g. "1 IN (SELECT 1)")
@@ -1305,8 +1321,8 @@ private function resolveExprType(Expr\Expr $expr, ?AnalyserConditionTypeEnum $co
 			case Expr\ExprTypeEnum::LIKE:
 				assert($expr instanceof Expr\Like);
 				// TODO: handle $condition
-				$expressionResult = $this->resolveExprType($expr->expression);
-				$patternResult = $this->resolveExprType($expr->pattern);
+				$expressionResult = $this->resolveExprType($expr->expression, null, $isNonEmptyAggResultSet);
+				$patternResult = $this->resolveExprType($expr->pattern, null, $isNonEmptyAggResultSet);
 				// TODO: check for valid escape char expressions.
 				// For example "ESCAPE IF(0, 'a', 'b')" seems to work, but "ESCAPE IF(id = id, 'a', 'b')" doesn't.
 				$escapeCharResult = $expr->escapeChar !== null
@@ -1379,7 +1395,11 @@ private function resolveExprType(Expr\Expr $expr, ?AnalyserConditionTypeEnum $co
 				foreach ($arguments as $arg) {
 					$innerCondition = $innerConditions[$position] ?? null;
 					$position++;
-					$resolvedArguments[] = $resolvedArg = $this->resolveExprType($arg, $innerCondition);
+					$resolvedArguments[] = $resolvedArg = $this->resolveExprType(
+						$arg,
+						$innerCondition,
+						$isNonEmptyAggResultSet,
+					);
 
 					if ($resolvedArg->type::getTypeEnum() === Schema\DbType\DbTypeEnum::TUPLE) {
 						$this->errors[] = AnalyserErrorBuilder::createInvalidFunctionArgumentError(
@@ -1398,7 +1418,12 @@ private function resolveExprType(Expr\Expr $expr, ?AnalyserConditionTypeEnum $co
 
 				if ($functionInfo !== null) {
 					try {
-						return $functionInfo->getReturnType($expr, $resolvedArguments, $condition);
+						return $functionInfo->getReturnType(
+							$expr,
+							$resolvedArguments,
+							$condition,
+							$isNonEmptyAggResultSet,
+						);
 					} catch (AnalyserException $e) {
 						$this->errors[] = $e->toAnalyserError();
 					}
@@ -1413,7 +1438,7 @@ private function resolveExprType(Expr\Expr $expr, ?AnalyserConditionTypeEnum $co
 
 				// TODO: handle $condition
 				if ($expr->compareValue) {
-					$field = $this->resolveExprType($expr->compareValue);
+					$field = $this->resolveExprType($expr->compareValue, null, $isNonEmptyAggResultSet);
 					$this->checkNotTuple($field->type);
 				}
 
@@ -1422,12 +1447,17 @@ private function resolveExprType(Expr\Expr $expr, ?AnalyserConditionTypeEnum $co
 				foreach ($expr->conditions as $caseCondition) {
 					$field = $this->resolveExprType($caseCondition->when);
 					$this->checkNotTuple($field->type);
-					$subresults[] = $field = $this->resolveExprType($caseCondition->then);
+					$subresults[] = $field = $this->resolveExprType(
+						$caseCondition->then,
+						null,
+						$isNonEmptyAggResultSet,
+					);
 					$this->checkNotTuple($field->type);
 				}
 
+				// TODO: CASE with no else may be nullable
 				if ($expr->else) {
-					$subresults[] = $field = $this->resolveExprType($expr->else);
+					$subresults[] = $field = $this->resolveExprType($expr->else, null, $isNonEmptyAggResultSet);
 					$this->checkNotTuple($field->type);
 				}
 
@@ -1446,6 +1476,7 @@ private function resolveExprType(Expr\Expr $expr, ?AnalyserConditionTypeEnum $co
 			case Expr\ExprTypeEnum::EXISTS:
 				assert($expr instanceof Expr\Exists);
 				// TODO: handle $condition
+				// TODO: handle $isNonEmptyAggResultSet
 				$this->getSubqueryAnalyser($expr->subquery)->analyse();
 
 				return new ExprTypeResult(
@@ -1455,6 +1486,7 @@ private function resolveExprType(Expr\Expr $expr, ?AnalyserConditionTypeEnum $co
 			case Expr\ExprTypeEnum::ASSIGNMENT:
 				assert($expr instanceof Expr\Assignment);
 				// TODO: handle $condition
+				// TODO: handle $isNonEmptyAggResultSet
 				$this->runWithDifferentFieldBehavior(
 					ColumnResolverFieldBehaviorEnum::ASSIGNMENT,
 					fn () => $this->resolveExprType($expr->target),
@@ -1469,7 +1501,7 @@ private function resolveExprType(Expr\Expr $expr, ?AnalyserConditionTypeEnum $co
 				);
 			case Expr\ExprTypeEnum::COLLATE:
 				assert($expr instanceof Expr\Collate);
-				$subresult = $this->resolveExprType($expr->expression, $condition);
+				$subresult = $this->resolveExprType($expr->expression, $condition, $isNonEmptyAggResultSet);
 
 				return new ExprTypeResult(
 					new Schema\DbType\VarcharType(),

diff --git a/src/Database/FunctionInfo/Avg.php b/src/Database/FunctionInfo/Avg.php
@@ -68,6 +68,7 @@ public function getReturnType(
 		FunctionCall $functionCall,
 		array $argumentTypes,
 		?AnalyserConditionTypeEnum $condition,
+		bool $isNonEmptyAggResultSet,
 	): ExprTypeResult {
 		$arg = reset($argumentTypes);
 		assert($arg instanceof ExprTypeResult);
@@ -78,6 +79,6 @@ public function getReturnType(
 			default => new DecimalType(),
 		};
 
-		return new ExprTypeResult($type, true);
+		return new ExprTypeResult($type, ! $isNonEmptyAggResultSet || $arg->isNullable);
 	}
 }
diff --git a/src/Database/FunctionInfo/Cast.php b/src/Database/FunctionInfo/Cast.php
@@ -51,6 +51,7 @@ public function getReturnType(
 		FunctionCall $functionCall,
 		array $argumentTypes,
 		?AnalyserConditionTypeEnum $condition,
+		bool $isNonEmptyAggResultSet,
 	): ExprTypeResult {
 		assert($functionCall instanceof CastFunctionCall);
 		$exprType = $argumentTypes[0];

diff --git a/src/Database/FunctionInfo/CeilFloor.php b/src/Database/FunctionInfo/CeilFloor.php
@@ -60,6 +60,7 @@ public function getReturnType(
 		FunctionCall $functionCall,
 		array $argumentTypes,
 		?AnalyserConditionTypeEnum $condition,
+		bool $isNonEmptyAggResultSet,
 	): ExprTypeResult {
 		$value = $argumentTypes[0];
 

diff --git a/src/Database/FunctionInfo/Coalesce.php b/src/Database/FunctionInfo/Coalesce.php
@@ -84,6 +84,7 @@ public function getReturnType(
 		FunctionCall $functionCall,
 		array $argumentTypes,
 		?AnalyserConditionTypeEnum $condition,
+		bool $isNonEmptyAggResultSet,
 	): ExprTypeResult {
 		$leftType = $argumentTypes[0]->type;
 		$isNullable = $argumentTypes[0]->isNullable;

diff --git a/src/Database/FunctionInfo/Concat.php b/src/Database/FunctionInfo/Concat.php
@@ -73,6 +73,7 @@ public function getReturnType(
 		FunctionCall $functionCall,
 		array $argumentTypes,
 		?AnalyserConditionTypeEnum $condition,
+		bool $isNonEmptyAggResultSet,
 	): ExprTypeResult {
 		$leftType = $this->concatOneType($argumentTypes[0]->type);
 		$isNullable = $argumentTypes[0]->isNullable;

diff --git a/src/Database/FunctionInfo/Count.php b/src/Database/FunctionInfo/Count.php
@@ -43,6 +43,7 @@ public function getReturnType(
 		FunctionCall $functionCall,
 		array $argumentTypes,
 		?AnalyserConditionTypeEnum $condition,
+		bool $isNonEmptyAggResultSet,
 	): ExprTypeResult {
 		return new ExprTypeResult(new IntType(), false);
 	}

diff --git a/src/Database/FunctionInfo/Curdate.php b/src/Database/FunctionInfo/Curdate.php
@@ -56,6 +56,7 @@ public function getReturnType(
 		FunctionCall $functionCall,
 		array $argumentTypes,
 		?AnalyserConditionTypeEnum $condition,
+		bool $isNonEmptyAggResultSet,
 	): ExprTypeResult {
 		$kb = match ($condition) {
 			null => null,

diff --git a/src/Database/FunctionInfo/Date.php b/src/Database/FunctionInfo/Date.php
@@ -59,6 +59,7 @@ public function getReturnType(
 		FunctionCall $functionCall,
 		array $argumentTypes,
 		?AnalyserConditionTypeEnum $condition,
+		bool $isNonEmptyAggResultSet,
 	): ExprTypeResult {
 		$date = $argumentTypes[0];
 		$isNullable = $date->isNullable || $date->type::getTypeEnum() !== DbTypeEnum::DATETIME;

diff --git a/src/Database/FunctionInfo/DateAddSub.php b/src/Database/FunctionInfo/DateAddSub.php
@@ -44,6 +44,7 @@ public function getReturnType(
 		FunctionCall $functionCall,
 		array $argumentTypes,
 		?AnalyserConditionTypeEnum $condition,
+		bool $isNonEmptyAggResultSet,
 	): ExprTypeResult {
 		$date = $argumentTypes[0];
 		$interval = $argumentTypes[1];

diff --git a/src/Database/FunctionInfo/DateFormat.php b/src/Database/FunctionInfo/DateFormat.php
@@ -60,6 +60,7 @@ public function getReturnType(
 		FunctionCall $functionCall,
 		array $argumentTypes,
 		?AnalyserConditionTypeEnum $condition,
+		bool $isNonEmptyAggResultSet,
 	): ExprTypeResult {
 		$date = $argumentTypes[0];
 		$format = $argumentTypes[1];

diff --git a/src/Database/FunctionInfo/FoundRows.php b/src/Database/FunctionInfo/FoundRows.php
@@ -58,6 +58,7 @@ public function getReturnType(
 		FunctionCall $functionCall,
 		array $argumentTypes,
 		?AnalyserConditionTypeEnum $condition,
+		bool $isNonEmptyAggResultSet,
 	): ExprTypeResult {
 		return new ExprTypeResult(new IntType(), false);
 	}

diff --git a/src/Database/FunctionInfo/FunctionInfo.php b/src/Database/FunctionInfo/FunctionInfo.php
@@ -35,5 +35,6 @@ public function getReturnType(
 		FunctionCall $functionCall,
 		array $argumentTypes,
 		?AnalyserConditionTypeEnum $condition,
+		bool $isNonEmptyAggResultSet,
 	): ExprTypeResult;
 }
diff --git a/src/Database/FunctionInfo/GroupConcat.php b/src/Database/FunctionInfo/GroupConcat.php
@@ -43,7 +43,14 @@ public function getReturnType(
 		FunctionCall $functionCall,
 		array $argumentTypes,
 		?AnalyserConditionTypeEnum $condition,
+		bool $isNonEmptyAggResultSet,
 	): ExprTypeResult {
-		return new ExprTypeResult(new VarcharType(), true);
+		$isAnyArgNullable = false;
+
+		foreach ($argumentTypes as $argumentType) {
+			$isAnyArgNullable = $isAnyArgNullable || $argumentType->isNullable;
+		}
+
+		return new ExprTypeResult(new VarcharType(), ! $isNonEmptyAggResultSet || $isAnyArgNullable);
 	}
 }
diff --git a/src/Database/FunctionInfo/IfFunction.php b/src/Database/FunctionInfo/IfFunction.php
@@ -55,6 +55,7 @@ public function getReturnType(
 		FunctionCall $functionCall,
 		array $argumentTypes,
 		?AnalyserConditionTypeEnum $condition,
+		bool $isNonEmptyAggResultSet,
 	): ExprTypeResult {
 		$then = $argumentTypes[1];
 		$else = $argumentTypes[2];

diff --git a/src/Database/FunctionInfo/LowerUpper.php b/src/Database/FunctionInfo/LowerUpper.php
@@ -57,6 +57,7 @@ public function getReturnType(
 		FunctionCall $functionCall,
 		array $argumentTypes,
 		?AnalyserConditionTypeEnum $condition,
+		bool $isNonEmptyAggResultSet,
 	): ExprTypeResult {
 		$arg = $argumentTypes[0];
 

diff --git a/src/Database/FunctionInfo/MaxMin.php b/src/Database/FunctionInfo/MaxMin.php
@@ -64,10 +64,11 @@ public function getReturnType(
 		FunctionCall $functionCall,
 		array $argumentTypes,
 		?AnalyserConditionTypeEnum $condition,
+		bool $isNonEmptyAggResultSet,
 	): ExprTypeResult {
 		$arg = reset($argumentTypes);
 		assert($arg instanceof ExprTypeResult);
 
-		return new ExprTypeResult($arg->type, true);
+		return new ExprTypeResult($arg->type, ! $isNonEmptyAggResultSet || $arg->isNullable);
 	}
 }
diff --git a/src/Database/FunctionInfo/Now.php b/src/Database/FunctionInfo/Now.php
@@ -83,6 +83,7 @@ public function getReturnType(
 		FunctionCall $functionCall,
 		array $argumentTypes,
 		?AnalyserConditionTypeEnum $condition,
+		bool $isNonEmptyAggResultSet,
 	): ExprTypeResult {
 		$kb = match ($condition) {
 			null => null,

diff --git a/src/Database/FunctionInfo/Round.php b/src/Database/FunctionInfo/Round.php
@@ -60,6 +60,7 @@ public function getReturnType(
 		FunctionCall $functionCall,
 		array $argumentTypes,
 		?AnalyserConditionTypeEnum $condition,
+		bool $isNonEmptyAggResultSet,
 	): ExprTypeResult {
 		$valueType = $argumentTypes[0];
 		$digitsType = $argumentTypes[1] ?? null;

diff --git a/src/Database/FunctionInfo/Sum.php b/src/Database/FunctionInfo/Sum.php
@@ -68,6 +68,7 @@ public function getReturnType(
 		FunctionCall $functionCall,
 		array $argumentTypes,
 		?AnalyserConditionTypeEnum $condition,
+		bool $isNonEmptyAggResultSet,
 	): ExprTypeResult {
 		$arg = reset($argumentTypes);
 		assert($arg instanceof ExprTypeResult);
@@ -78,6 +79,6 @@ public function getReturnType(
 			default => new DecimalType(),
 		};
 
-		return new ExprTypeResult($type, true);
+		return new ExprTypeResult($type, ! $isNonEmptyAggResultSet || $arg->isNullable);
 	}
 }
diff --git a/src/Database/FunctionInfo/Trim.php b/src/Database/FunctionInfo/Trim.php
@@ -43,6 +43,7 @@ public function getReturnType(
 		FunctionCall $functionCall,
 		array $argumentTypes,
 		?AnalyserConditionTypeEnum $condition,
+		bool $isNonEmptyAggResultSet,
 	): ExprTypeResult {
 		$isNullable = false;
 

diff --git a/src/Database/FunctionInfo/Value.php b/src/Database/FunctionInfo/Value.php
@@ -66,6 +66,7 @@ public function getReturnType(
 		FunctionCall $functionCall,
 		array $argumentTypes,
 		?AnalyserConditionTypeEnum $condition,
+		bool $isNonEmptyAggResultSet,
 	): ExprTypeResult {
 		$col = $argumentTypes[0];
 		// VALUE(...) can be used in SELECT as well, in which case it always returns null.