Merge cockroachdb#36972

36972: exec: some cleanup and small perf wins r=jordanlewis a=jordanlewis

See individual commits for details. Just did a bunch of bounds check elimination stuff while I was on the plane. Also had an interesting observation which is that it's unnecessary to have pointer receivers for `Next` a lot of the time, which speeds things up by removing redundant checks from the compiler which couldn't prove in the pointer receiver case that data from the struct hadn't gotten modified by another goroutine.

And fixed a bug in hash join planning that caused us to fail to run some TPCH queries.

Co-authored-by: Jordan Lewis <jordanthelewis@gmail.com>

Makefile

@@ -1402,6 +1402,7 @@ pkg/sql/exec/distinct.eg.go: pkg/sql/exec/distinct_tmpl.go
 pkg/sql/exec/hashjoiner.eg.go: pkg/sql/exec/hashjoiner_tmpl.go
 pkg/sql/exec/mergejoiner.eg.go: pkg/sql/exec/mergejoiner_tmpl.go
 pkg/sql/exec/quicksort.eg.go: pkg/sql/exec/quicksort_tmpl.go
+pkg/sql/exec/rowstovec.eg.go: pkg/sql/exec/rowstovec_tmpl.go
 pkg/sql/exec/sort.eg.go: pkg/sql/exec/sort_tmpl.go
 pkg/sql/exec/sum_agg.eg.go: pkg/sql/exec/sum_agg_tmpl.go
 pkg/sql/exec/tuples_differ.eg.go: pkg/sql/exec/tuples_differ_tmpl.go

pkg/sql/distsqlrun/column_exec_setup.go

@@ -236,6 +236,10 @@ func newColOperator(
 		leftTypes := conv.FromColumnTypes(spec.Input[0].ColumnTypes)
 		rightTypes := conv.FromColumnTypes(spec.Input[1].ColumnTypes)
 
+		columnTypes = make([]semtypes.T, len(leftTypes)+len(rightTypes))
+		copy(columnTypes, spec.Input[0].ColumnTypes)
+		copy(columnTypes[len(leftTypes):], spec.Input[1].ColumnTypes)
+
 		nLeftCols := uint32(len(leftTypes))
 		nRightCols := uint32(len(rightTypes))
 

pkg/sql/exec/bool_vec_to_sel.go

@@ -39,7 +39,8 @@ func (p *boolVecToSelOp) Next(ctx context.Context) coldata.Batch {
 	// exhausted.
 	for {
 		batch := p.input.Next(ctx)
-		if batch.Length() == 0 {
+		n := batch.Length()
+		if n == 0 {
 			return batch
 		}
 		outputCol := p.outputCol
@@ -49,23 +50,30 @@ func (p *boolVecToSelOp) Next(ctx context.Context) coldata.Batch {
 		// Note that, if the input already had a selection vector, the output
 		// selection vector will be a subset of the input selection vector.
 		idx := uint16(0)
-		n := batch.Length()
 		if sel := batch.Selection(); sel != nil {
-			for s := uint16(0); s < n; s++ {
+			sel = sel[:n]
+			for s := range sel {
 				i := sel[s]
+				var inc uint16
+				// This form is transformed into a data dependency by the compiler,
+				// avoiding an expensive conditional branch.
 				if outputCol[i] {
-					sel[idx] = i
-					idx++
+					inc = 1
 				}
+				sel[idx] = i
+				idx += inc
 			}
 		} else {
 			batch.SetSelection(true)
 			sel := batch.Selection()
-			for i := uint16(0); i < n; i++ {
+			for i := range outputCol[:n] {
+				var inc uint16
+				// Ditto above: replace a conditional with a data dependency.
 				if outputCol[i] {
-					sel[idx] = i
-					idx++
+					inc = 1
 				}
+				sel[idx] = uint16(i)
+				idx += inc
 			}
 		}
 
@@ -74,9 +82,7 @@ func (p *boolVecToSelOp) Next(ctx context.Context) coldata.Batch {
 		}
 
 		// Zero our output column for next time.
-		for i := range p.outputCol {
-			p.outputCol[i] = false
-		}
+		copy(p.outputCol, zeroBoolVec)
 
 		batch.SetLength(idx)
 		return batch

pkg/sql/exec/count_agg.go

@@ -73,7 +73,7 @@ func (a *countAgg) Compute(b coldata.Batch, _ []uint32) {
 			a.vec[a.curIdx]++
 		}
 	} else {
-		for i := uint16(0); i < inputLen; i++ {
+		for i := range a.groups[:inputLen] {
 			x := 0
 			if a.groups[i] {
 				x = 1

pkg/sql/exec/distinct_tmpl.go

@@ -180,6 +180,7 @@ func (p *sortedDistinct_TYPEOp) Next(ctx context.Context) coldata.Batch {
 	// We always output the first row.
 	lastVal := p.lastVal
 	sel := batch.Selection()
+	startIdx := uint16(0)
 	if !p.foundFirstRow {
 		if sel != nil {
 			lastVal = col[sel[0]]
@@ -188,31 +189,32 @@ func (p *sortedDistinct_TYPEOp) Next(ctx context.Context) coldata.Batch {
 			lastVal = col[0]
 			outputCol[0] = true
 		}
-	}
-
-	startIdx := uint16(0)
-	if !p.foundFirstRow {
 		startIdx = 1
+		p.foundFirstRow = true
+		if batch.Length() == 1 {
+			p.lastVal = lastVal
+			return batch
+		}
 	}
 
 	n := batch.Length()
 	if sel != nil {
 		// Bounds check elimination.
 		sel = sel[startIdx:n]
 		for _, i := range sel {
-			_INNER_LOOP(int(i), lastVal, col, outputCol)
+			_CHECK_DISTINCT(int(i), lastVal, col, outputCol)
 		}
 	} else {
 		// Bounds check elimination.
 		col = col[startIdx:n]
 		outputCol = outputCol[startIdx:n]
+		_ = outputCol[len(col)-1]
 		for i := range col {
-			_INNER_LOOP(i, lastVal, col, outputCol)
+			_CHECK_DISTINCT(i, lastVal, col, outputCol)
 		}
 	}
 
 	p.lastVal = lastVal
-	p.foundFirstRow = true
 
 	return batch
 }
@@ -230,25 +232,22 @@ func (p partitioner_TYPE) partition(colVec coldata.Vec, outputCol []bool, n uint
 	outputCol = outputCol[1:n]
 	col = col[1:n]
 	for i := range col {
-		v := col[i]
-		var unique bool
-		_ASSIGN_NE("unique", "v", "lastVal")
-		outputCol[i] = outputCol[i] || unique
-		lastVal = v
+		_CHECK_DISTINCT(i, lastVal, col, outputCol)
 	}
 }
 
 // {{end}}
 
 // {{/*
-func _INNER_LOOP(i int, lastVal _GOTYPE, col []interface{}, outputCol []bool) { // */}}
+// _CHECK_DISTINCT retrieves the value at the ith index of col, compares it
+// to the passed in lastVal, and sets the ith value of outputCol to true if the
+// compared values were distinct.
+func _CHECK_DISTINCT(i int, lastVal _GOTYPE, col []_GOTYPE, outputCol []bool) { // */}}
 
-	// {{define "innerLoop"}}
+	// {{define "checkDistinct"}}
 	v := col[i]
-	// Note that not inlining this unique var actually makes a non-trivial
-	// performance difference.
 	var unique bool
-	_ASSIGN_NE("unique", "v", "lastVal")
+	_ASSIGN_NE(unique, v, lastVal)
 	outputCol[i] = outputCol[i] || unique
 	lastVal = v
 	// {{end}}

pkg/sql/exec/execgen/cmd/execgen/distinct_gen.go

@@ -39,10 +39,10 @@ func genDistinctOps(wr io.Writer) error {
 	s = strings.Replace(s, "_TemplateType", "{{.LTyp}}", -1)
 
 	assignNeRe := regexp.MustCompile(`_ASSIGN_NE\((.*),(.*),(.*)\)`)
-	s = assignNeRe.ReplaceAllString(s, "{{.Assign $1 $2 $3}}")
+	s = assignNeRe.ReplaceAllString(s, `{{.Assign "$1" "$2" "$3"}}`)
 
-	innerLoopRe := regexp.MustCompile(`_INNER_LOOP\(.*\)`)
-	s = innerLoopRe.ReplaceAllString(s, `{{template "innerLoop" .}}`)
+	innerLoopRe := regexp.MustCompile(`_CHECK_DISTINCT\(.*\)`)
+	s = innerLoopRe.ReplaceAllString(s, `{{template "checkDistinct" .}}`)
 
 	// Now, generate the op, from the template.
 	tmpl, err := template.New("distinct_op").Parse(s)

pkg/sql/exec/execgen/cmd/execgen/projection_ops_gen.go

@@ -55,22 +55,26 @@ type {{template "opRConstName" .}} struct {
 	outputIdx int
 }
 
-func (p *{{template "opRConstName" .}}) Next(ctx context.Context) coldata.Batch {
+func (p {{template "opRConstName" .}}) Next(ctx context.Context) coldata.Batch {
 	batch := p.input.Next(ctx)
+	n := batch.Length()
+	if n == 0 {
+		return batch
+	}
 	if p.outputIdx == batch.Width() {
 		batch.AppendCol(types.{{.RetTyp}})
 	}
+	col := batch.ColVec(p.colIdx).{{.LTyp}}()[:coldata.BatchSize]
 	projCol := batch.ColVec(p.outputIdx).{{.RetTyp}}()[:coldata.BatchSize]
-	coldata := batch.ColVec(p.colIdx).{{.LTyp}}()[:coldata.BatchSize]
-	n := batch.Length()
 	if sel := batch.Selection(); sel != nil {
 		for _, i := range sel {
-			{{(.Assign "projCol[i]" "coldata[i]" "p.constArg")}}
+			{{(.Assign "projCol[i]" "col[i]" "p.constArg")}}
 		}
 	} else {
-		coldata = coldata[:n]
-		for i := range coldata {
-			{{(.Assign "projCol[i]" "coldata[i]" "p.constArg")}}
+		col = col[:n]
+		_ = projCol[len(col)-1]
+		for i := range col {
+			{{(.Assign "projCol[i]" "col[i]" "p.constArg")}}
 		}
 	}
 	return batch
@@ -89,20 +93,24 @@ type {{template "opLConstName" .}} struct {
 	outputIdx int
 }
 
-func (p *{{template "opLConstName" .}}) Next(ctx context.Context) coldata.Batch {
+func (p {{template "opLConstName" .}}) Next(ctx context.Context) coldata.Batch {
 	batch := p.input.Next(ctx)
+	n := batch.Length()
+	if n == 0 {
+		return batch
+	}
 	if p.outputIdx == batch.Width() {
 		batch.AppendCol(types.{{.RetTyp}})
 	}
 	projCol := batch.ColVec(p.outputIdx).{{.RetTyp}}()[:coldata.BatchSize]
 	coldata := batch.ColVec(p.colIdx).{{.RTyp}}()[:coldata.BatchSize]
-	n := batch.Length()
 	if sel := batch.Selection(); sel != nil {
 		for _, i := range sel {
 			{{(.Assign "projCol[i]" "p.constArg" "coldata[i]")}}
 		}
 	} else {
 		coldata = coldata[:n]
+		_ = projCol[len(coldata)-1]
 		for i := range coldata {
 			{{(.Assign "projCol[i]" "p.constArg" "coldata[i]")}}
 		}
@@ -123,21 +131,26 @@ type {{template "opName" .}} struct {
 	outputIdx int
 }
 
-func (p *{{template "opName" .}}) Next(ctx context.Context) coldata.Batch {
+func (p {{template "opName" .}}) Next(ctx context.Context) coldata.Batch {
 	batch := p.input.Next(ctx)
+	n := batch.Length()
+	if n == 0 {
+		return batch
+	}
 	if p.outputIdx == batch.Width() {
 		batch.AppendCol(types.{{.RetTyp}})
 	}
 	projCol := batch.ColVec(p.outputIdx).{{.RetTyp}}()[:coldata.BatchSize]
 	col1 := batch.ColVec(p.col1Idx).{{.LTyp}}()[:coldata.BatchSize]
 	col2 := batch.ColVec(p.col2Idx).{{.RTyp}}()[:coldata.BatchSize]
-	n := batch.Length()
 	if sel := batch.Selection(); sel != nil {
 		for _, i := range sel {
 			{{(.Assign "projCol[i]" "col1[i]" "col2[i]")}}
 		}
 	} else {
 		col1 = col1[:n]
+		_ = projCol[len(col1)-1]
+		_ = col2[len(col1)-1]
 		for i := range col1 {
 			{{(.Assign "projCol[i]" "col1[i]" "col2[i]")}}
 		}

pkg/sql/exec/rowstovec_tmpl.go

@@ -50,18 +50,18 @@ func _ROWS_TO_COL_VEC(
 	rows sqlbase.EncDatumRows, vec coldata.Vec, columnIdx int, alloc *sqlbase.DatumAlloc,
 ) error { // */}}
 	// {{define "rowsToColVec"}}
-	nRows := uint16(len(rows))
 	col := vec._TemplateType()
 	datumToPhysicalFn := conv.GetDatumToPhysicalFn(columnType)
-	for i := uint16(0); i < nRows; i++ {
-		if rows[i][columnIdx].Datum == nil {
-			if err := rows[i][columnIdx].EnsureDecoded(columnType, alloc); err != nil {
+	for i := range rows {
+		row := rows[i]
+		if row[columnIdx].Datum == nil {
+			if err := row[columnIdx].EnsureDecoded(columnType, alloc); err != nil {
 				return err
 			}
 		}
-		datum := rows[i][columnIdx].Datum
+		datum := row[columnIdx].Datum
 		if datum == tree.DNull {
-			vec.SetNull(i)
+			vec.SetNull(uint16(i))
 		} else {
 			v, err := datumToPhysicalFn(datum)
 			if err != nil {