Merge #89544

89544: opt: remove TODOs related to UDFs named "count" r=mgartner a=mgartner Related to #89543 Release note: None Co-authored-by: Marcus Gartner <[email protected]>
cockroachdb · Oct 7, 2022 · fa47f7b · fa47f7b
2 parents 44c714c + 2bf8569
commit fa47f7b
Showing 1 changed file with 3 additions and 6 deletions.
diff --git a/pkg/sql/opt/optbuilder/scope.go b/pkg/sql/opt/optbuilder/scope.go
@@ -1534,6 +1534,9 @@ func (s *scope) replaceCount(
 	}
 	f.Exprs[0] = vn
 
+	// It is ok to use string equality here, even if there is a UDF named
+	// "count" because UDFs cannot be aggregate functions. This code path is
+	// only executed for aggregate functions.
 	if strings.EqualFold(def.Name, "count") && f.Type == 0 {
 		if _, ok := vn.(tree.UnqualifiedStar); ok {
 			if f.Filter != nil {
@@ -1548,9 +1551,6 @@ func (s *scope) replaceCount(
 				e := &cpy
 				e.Exprs = tree.Exprs{tree.DBoolTrue}
 
-				// TODO(mgartner): What happens if a user defines a UDF named
-				// "count"? We might need to resolve the function first, not
-				// just rely on string equality to "count" above.
 				newDef, err := e.Func.Resolve(s.builder.ctx, s.builder.semaCtx.SearchPath, nil /* resolver */)
 				if err != nil {
 					panic(err)
@@ -1577,9 +1577,6 @@ func (s *scope) replaceCount(
 			if _, err := e.TypeCheck(s.builder.ctx, &semaCtx, types.Any); err != nil {
 				panic(err)
 			}
-			// TODO(mgartner): What happens if a user defines a UDF named
-			// "count"? We might need to resolve the function first, not just
-			// rely on string equality to "count" above.
 			newDef, err := e.Func.Resolve(s.builder.ctx, s.builder.semaCtx.SearchPath, nil /* resolver */)
 			if err != nil {
 				panic(err)