From 6ecec23d43589cb3f229a106e33f2f5b03ff636d Mon Sep 17 00:00:00 2001
From: Rebecca Taft <becca@cockroachlabs.com>
Date: Thu, 26 Mar 2020 15:13:29 -0500
Subject: [PATCH] sql: fix type checking code for aggregate functions

Prior to this commit, any aggregate function that had an argument with
unknown type was replaced with NULL. This is incorrect for scalar
aggregates when the input relation has multiple rows, because after
replacement, the query result has the same number of rows as the input
relation. It should instead be reduced to a single row.

This commit fixes the issue by avoiding replacing the aggregate with NULL.

Note that for many aggregates, this change results in an ambiguous function
error since the type checking code cannot choose which overload is correct
for the unknown type. This is different behavior than Postgres, which
defaults to type "text".

Fixes #46196

Release justification: this is a low risk, high benefit change to existing
functionality.
Release note (bug fix): fixed an incorrect query result that could occur
when a scalar aggregate was called with a null input.
---
 pkg/sql/logictest/testdata/logic_test/typing |  7 ++++++
 pkg/sql/opt/optbuilder/testdata/aggregate    | 23 ++++++++++++++++++++
 pkg/sql/sem/tree/type_check.go               |  7 +++---
 3 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/pkg/sql/logictest/testdata/logic_test/typing b/pkg/sql/logictest/testdata/logic_test/typing
index 47a48b5102ea..29ffac49ef69 100644
--- a/pkg/sql/logictest/testdata/logic_test/typing
+++ b/pkg/sql/logictest/testdata/logic_test/typing
@@ -221,3 +221,10 @@ query I
 SELECT NULLIF(0, NULL) + NULLIF(0, NULL)
 ----
 0
+
+# Regression test for #46196.
+query error ambiguous call: max\(unknown\)
+SELECT MAX(t0.c0) FROM (VALUES (NULL), (NULL)) t0(c0)
+
+query error ambiguous call: max\(unknown\)
+SELECT MAX(NULL) FROM (VALUES (NULL), (NULL)) t0(c0)
diff --git a/pkg/sql/opt/optbuilder/testdata/aggregate b/pkg/sql/opt/optbuilder/testdata/aggregate
index a683a3e073a9..ec0b9366e774 100644
--- a/pkg/sql/opt/optbuilder/testdata/aggregate
+++ b/pkg/sql/opt/optbuilder/testdata/aggregate
@@ -3839,3 +3839,26 @@ project
                      │              └── b:3
                      └── projections
                           └── (max:4, unnest:1) [as="?column?":6]
+
+# Regression test for #46196.
+build
+SELECT MAX(t0.c0) FROM (VALUES (NULL), (NULL)) t0(c0);
+----
+error (42725): ambiguous call: max(unknown), candidates are:
+max(int) -> int
+max(float) -> float
+max(decimal) -> decimal
+max(date) -> date
+max(timestamp) -> timestamp
+max(interval) -> interval
+max(string) -> string
+max(bytes) -> bytes
+max(timestamptz) -> timestamptz
+max(oid) -> oid
+max(uuid) -> uuid
+max(inet) -> inet
+max(time) -> time
+max(timetz) -> timetz
+max(jsonb) -> jsonb
+max(varbit) -> varbit
+max(bool) -> bool
diff --git a/pkg/sql/sem/tree/type_check.go b/pkg/sql/sem/tree/type_check.go
index 792eabeb8c4b..f26566e96a58 100644
--- a/pkg/sql/sem/tree/type_check.go
+++ b/pkg/sql/sem/tree/type_check.go
@@ -848,9 +848,10 @@ func (expr *FuncExpr) TypeCheck(ctx *SemaContext, desired *types.T) (TypedExpr,
 	}
 
 	// Return NULL if at least one overload is possible, no overload accepts
-	// NULL arguments, the function isn't a generator builtin, and NULL is given
-	// as an argument.
-	if !def.NullableArgs && def.FunctionProperties.Class != GeneratorClass {
+	// NULL arguments, the function isn't a generator or aggregate builtin, and
+	// NULL is given as an argument.
+	if !def.NullableArgs && def.FunctionProperties.Class != GeneratorClass &&
+		def.FunctionProperties.Class != AggregateClass {
 		for _, expr := range typedSubExprs {
 			if expr.ResolvedType().Family() == types.UnknownFamily {
 				return DNull, nil