From 8dffa4b38f108ec3e8fe28192eac43ac1c14a9ea Mon Sep 17 00:00:00 2001
From: Yahor Yuzefovich <yahor@cockroachlabs.com>
Date: Sat, 17 Oct 2020 10:50:01 -0700
Subject: [PATCH] colexec: clean up min_max template

We have recently removed some custom logic with min/max aggregate
templates because they are now correctly typed, and this allows us to
clean up the template a bit.

Release note: None
---
 .../execgen/cmd/execgen/min_max_agg_gen.go    | 67 +++++--------------
 pkg/sql/colexec/min_max_agg_tmpl.go           | 11 ++-
 2 files changed, 24 insertions(+), 54 deletions(-)

diff --git a/pkg/sql/colexec/execgen/cmd/execgen/min_max_agg_gen.go b/pkg/sql/colexec/execgen/cmd/execgen/min_max_agg_gen.go
index f4eb80423232..7f9ed891b4e6 100644
--- a/pkg/sql/colexec/execgen/cmd/execgen/min_max_agg_gen.go
+++ b/pkg/sql/colexec/execgen/cmd/execgen/min_max_agg_gen.go
@@ -18,37 +18,13 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 )
 
-type minMaxTmplInfo struct {
-	// Note that we embed the corresponding comparison overload (either LT or
-	// GT) into this struct (so that we could have access to methods like Set,
-	// Range, etc.) but also customize the result variables.
-	*lastArgWidthOverload
-	Agg string
-	// The following three fields have "Agg" prefix in order to not collide
-	// with the fields in lastArgWidthOverload, just to be safe.
-	AggRetGoTypeSlice string
-	AggRetGoType      string
-	AggRetVecMethod   string
-}
-
-// AggNameTitle returns the aggregation name in title case, e.g. "Min".
-func (a minMaxTmplInfo) AggNameTitle() string {
-	return strings.Title(a.Agg)
-}
-
-// Avoid unused warning for functions which are only used in templates.
-var _ = minMaxTmplInfo{}.AggNameTitle()
-
 const minMaxAggTmpl = "pkg/sql/colexec/min_max_agg_tmpl.go"
 
 func genMinMaxAgg(inputFileContents string, wr io.Writer) error {
-	// TODO(yuzefovich): clean up this file as per comments in #54080.
 	r := strings.NewReplacer(
-		"_AGG_TITLE", "{{.AggNameTitle}}",
-		"_AGG", "{{.Agg}}",
-		"_RET_GOTYPESLICE", "{{.AggRetGoTypeSlice}}",
-		"_RET_GOTYPE", "{{.AggRetGoType}}",
-		"_RET_TYPE", "{{.AggRetVecMethod}}",
+		"_AGG", "{{$agg}}",
+		"_GOTYPESLICE", "{{.GoTypeSliceName}}",
+		"_GOTYPE", "{{.GoType}}",
 		"_TYPE", "{{.VecMethod}}",
 		"TemplateType", "{{.VecMethod}}",
 	)
@@ -66,30 +42,19 @@ func genMinMaxAgg(inputFileContents string, wr io.Writer) error {
 	if err != nil {
 		return err
 	}
-
-	var tmplInfos []minMaxTmplInfo
-	for _, agg := range []string{"min", "max"} {
-		cmpOp := tree.LT
-		if agg == "max" {
-			cmpOp = tree.GT
-		}
-		for _, ov := range sameTypeComparisonOpToOverloads[cmpOp] {
-			for i := range ov.WidthOverloads {
-				widthOv := ov.WidthOverloads[i]
-				retGoTypeSlice := widthOv.GoTypeSliceName()
-				retGoType := widthOv.GoType
-				retVecMethod := widthOv.VecMethod
-				tmplInfos = append(tmplInfos, minMaxTmplInfo{
-					lastArgWidthOverload: widthOv,
-					Agg:                  agg,
-					AggRetGoTypeSlice:    retGoTypeSlice,
-					AggRetGoType:         retGoType,
-					AggRetVecMethod:      retVecMethod,
-				})
-			}
-		}
-	}
-	return tmpl.Execute(wr, tmplInfos)
+	return tmpl.Execute(wr, []struct {
+		Agg       string
+		Overloads []*oneArgOverload
+	}{
+		{
+			Agg:       "min",
+			Overloads: sameTypeComparisonOpToOverloads[tree.LT],
+		},
+		{
+			Agg:       "max",
+			Overloads: sameTypeComparisonOpToOverloads[tree.GT],
+		},
+	})
 }
 
 func init() {
diff --git a/pkg/sql/colexec/min_max_agg_tmpl.go b/pkg/sql/colexec/min_max_agg_tmpl.go
index 6f91e2e119ac..10ae1f96080b 100644
--- a/pkg/sql/colexec/min_max_agg_tmpl.go
+++ b/pkg/sql/colexec/min_max_agg_tmpl.go
@@ -110,6 +110,9 @@ func newMax_AGGKINDAggAlloc(
 }
 
 // {{range .}}
+// {{$agg := .Agg}}
+// {{range .Overloads}}
+// {{range .WidthOverloads}}
 
 type _AGG_TYPE_AGGKINDAgg struct {
 	// {{if eq "_AGGKIND" "Ordered"}}
@@ -121,9 +124,9 @@ type _AGG_TYPE_AGGKINDAgg struct {
 	// curAgg holds the running min/max, so we can index into the slice once per
 	// group, instead of on each iteration.
 	// NOTE: if foundNonNullForCurrentGroup is false, curAgg is undefined.
-	curAgg _RET_GOTYPE
+	curAgg _GOTYPE
 	// col points to the output vector we are updating.
-	col _RET_GOTYPESLICE
+	col _GOTYPESLICE
 	// vec is the same as col before conversion from coldata.Vec.
 	vec coldata.Vec
 	// foundNonNullForCurrentGroup tracks if we have seen any non-null values
@@ -140,7 +143,7 @@ func (a *_AGG_TYPE_AGGKINDAgg) Init(groups []bool, vec coldata.Vec) {
 	a.hashAggregateFuncBase.Init(groups, vec)
 	// {{end}}
 	a.vec = vec
-	a.col = vec._RET_TYPE()
+	a.col = vec._TYPE()
 	a.Reset()
 }
 
@@ -268,6 +271,8 @@ func (a *_AGG_TYPE_AGGKINDAggAlloc) newAggFunc() aggregateFunc {
 	return f
 }
 
+// {{end}}
+// {{end}}
 // {{end}}
 
 // {{/*