Merge cockroachdb#40760

40760: colexec: remove cols argument from runTests harness r=jordanlewis a=yuzefovich

Previously, we passed in cols argument into runTests to tell it
on which columns to compare the output with the expected. Most
of the time, cols were simply populated with 0 through batch.Width()-1
becoming redundant. In a few cases where an operator in test appends
a new column to the batch and wants to compare only the results on
of that column, I think it's just easier to include the input part
in the expected tuples. Now that argument is removed.

As a minor nicety, when we write a unit test for some new operator,
whenever we need to include the input into the expected tuples, it
means that the operator appends a new column to the batch, and this
can serve as a reminder of not short-circuit on an empty batch.

Release justification: Category 1: Non-production code changes.

Release note: None

Co-authored-by: Yahor Yuzefovich <[email protected]>

pkg/sql/colexec/aggregator_test.go

@@ -276,7 +276,7 @@ func TestAggregatorOneFunc(t *testing.T) {
 				t.Fatal(err)
 			}
 
-			out := newOpTestOutput(a, []int{0}, tc.expected)
+			out := newOpTestOutput(a, tc.expected)
 			// Explicitly reinitialize the aggregator with the given output batch
 			// size.
 			a.(*orderedAggregator).initWithInputAndOutputBatchSize(tc.batchSize, tc.outputBatchSize)
@@ -288,7 +288,7 @@ func TestAggregatorOneFunc(t *testing.T) {
 			t.Run(fmt.Sprintf("Randomized"), func(t *testing.T) {
 				for _, agg := range aggTypes {
 					t.Run(agg.name, func(t *testing.T) {
-						runTests(t, []tuples{tc.input}, tc.expected, unorderedVerifier, []int{0},
+						runTests(t, []tuples{tc.input}, tc.expected, unorderedVerifier,
 							func(input []Operator) (Operator, error) {
 								return agg.new(
 									input[0],
@@ -370,7 +370,7 @@ func TestAggregatorMultiFunc(t *testing.T) {
 				if err := tc.init(); err != nil {
 					t.Fatal(err)
 				}
-				runTests(t, []tuples{tc.input}, tc.expected, unorderedVerifier, []int{0, 1},
+				runTests(t, []tuples{tc.input}, tc.expected, unorderedVerifier,
 					func(input []Operator) (Operator, error) {
 						return agg.new(input[0], tc.colTypes, tc.aggFns, tc.groupCols, tc.aggCols, false /* isScalar */)
 					})
@@ -453,7 +453,6 @@ func TestAggregatorAllFunctions(t *testing.T) {
 					[]tuples{tc.input},
 					tc.expected,
 					orderedVerifier,
-					[]int{0, 1, 2, 3, 4, 5, 6, 7, 8}[:len(tc.expected[0])],
 					func(input []Operator) (Operator, error) {
 						return agg.new(input[0], tc.colTypes, tc.aggFns, tc.groupCols, tc.aggCols, false /* isScalar */)
 					})
@@ -820,12 +819,7 @@ func TestHashAggregator(t *testing.T) {
 		if err := tc.init(); err != nil {
 			t.Fatal(err)
 		}
-		nOutput := len(tc.aggCols)
-		cols := make([]int, nOutput)
-		for i := 0; i < nOutput; i++ {
-			cols[i] = i
-		}
-		runTests(t, []tuples{tc.input}, tc.expected, unorderedVerifier, cols, func(sources []Operator) (Operator, error) {
+		runTests(t, []tuples{tc.input}, tc.expected, unorderedVerifier, func(sources []Operator) (Operator, error) {
 			return NewHashAggregator(sources[0], tc.colTypes, tc.aggFns, tc.groupCols, tc.aggCols, false /* isScalar */)
 		})
 	}

pkg/sql/colexec/and_test.go

@@ -25,43 +25,43 @@ func TestAndOp(t *testing.T) {
 		// All variations of pairs separately first.
 		{
 			tuples:   tuples{{false, true}},
-			expected: tuples{{false}},
+			expected: tuples{{false, true, false}},
 		},
 		{
 			tuples:   tuples{{false, nil}},
-			expected: tuples{{false}},
+			expected: tuples{{false, nil, false}},
 		},
 		{
 			tuples:   tuples{{false, false}},
-			expected: tuples{{false}},
+			expected: tuples{{false, false, false}},
 		},
 		{
 			tuples:   tuples{{true, true}},
-			expected: tuples{{true}},
+			expected: tuples{{true, true, true}},
 		},
 		{
 			tuples:   tuples{{true, false}},
-			expected: tuples{{false}},
+			expected: tuples{{true, false, false}},
 		},
 		{
 			tuples:   tuples{{true, nil}},
-			expected: tuples{{nil}},
+			expected: tuples{{true, nil, nil}},
 			// The case of {nil, nil} is explicitly tested below.
 			skipAllNullsInjection: true,
 		},
 		{
 			tuples:   tuples{{nil, true}},
-			expected: tuples{{nil}},
+			expected: tuples{{nil, true, nil}},
 			// The case of {nil, nil} is explicitly tested below.
 			skipAllNullsInjection: true,
 		},
 		{
 			tuples:   tuples{{nil, false}},
-			expected: tuples{{false}},
+			expected: tuples{{nil, false, false}},
 		},
 		{
 			tuples:   tuples{{nil, nil}},
-			expected: tuples{{nil}},
+			expected: tuples{{nil, nil, nil}},
 		},
 		// Now all variations of pairs combined together to make sure that nothing
 		// funky going on with multiple tuples.
@@ -72,9 +72,9 @@ func TestAndOp(t *testing.T) {
 				{nil, true}, {nil, false}, {nil, nil},
 			},
 			expected: tuples{
-				{false}, {false}, {false},
-				{true}, {false}, {nil},
-				{nil}, {false}, {nil},
+				{false, true, false}, {false, nil, false}, {false, false, false},
+				{true, true, true}, {true, false, false}, {true, nil, nil},
+				{nil, true, nil}, {nil, false, false}, {nil, nil, nil},
 			},
 		},
 	}
@@ -94,7 +94,6 @@ func TestAndOp(t *testing.T) {
 			[]coltypes.T{coltypes.Bool, coltypes.Bool},
 			tc.expected,
 			orderedVerifier,
-			[]int{2},
 			func(input []Operator) (Operator, error) {
 				return NewAndOp(
 					input[0],

pkg/sql/colexec/bool_vec_to_sel_test.go

@@ -25,7 +25,7 @@ func TestBoolVecToSelOp(t *testing.T) {
 		},
 	}
 	for _, tc := range tcs {
-		runTests(t, []tuples{tc.tuples}, tc.expected, orderedVerifier, []int{0}, func(input []Operator) (Operator, error) {
+		runTests(t, []tuples{tc.tuples}, tc.expected, orderedVerifier, func(input []Operator) (Operator, error) {
 			return NewBoolVecToSelOp(input[0], 0), nil
 		})
 	}

pkg/sql/colexec/builtin_funcs_test.go

@@ -63,7 +63,7 @@ func TestBasicBuiltinFunctions(t *testing.T) {
 			inputCols:    []int{0},
 			inputTuples:  tuples{{1}, {-1}},
 			inputTypes:   []types.T{*types.Int},
-			outputTuples: tuples{{1}, {1}},
+			outputTuples: tuples{{1, 1}, {-1, 1}},
 			outputTypes:  []types.T{*types.Int, *types.Int},
 		},
 		{
@@ -72,7 +72,7 @@ func TestBasicBuiltinFunctions(t *testing.T) {
 			inputCols:    []int{0},
 			inputTuples:  tuples{{"Hello"}, {"The"}},
 			inputTypes:   []types.T{*types.String},
-			outputTuples: tuples{{5}, {3}},
+			outputTuples: tuples{{"Hello", 5}, {"The", 3}},
 			outputTypes:  []types.T{*types.String, *types.Int},
 		},
 	}
@@ -81,7 +81,7 @@ func TestBasicBuiltinFunctions(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
-			runTests(t, []tuples{tc.inputTuples}, tc.outputTuples, orderedVerifier, []int{1},
+			runTests(t, []tuples{tc.inputTuples}, tc.outputTuples, orderedVerifier,
 				func(input []Operator) (Operator, error) {
 					expr, err := parser.ParseExpr(tc.expr)
 					if err != nil {

pkg/sql/colexec/cast_test.go

@@ -93,9 +93,9 @@ func TestRandomizedCast(t *testing.T) {
 					}
 				}
 				input = append(input, tuple{c.fromPhysType(fromDatum)})
-				output = append(output, tuple{c.toPhysType(toDatum)})
+				output = append(output, tuple{c.fromPhysType(fromDatum), c.toPhysType(toDatum)})
 			}
-			runTests(t, []tuples{input}, output, orderedVerifier, []int{1},
+			runTests(t, []tuples{input}, output, orderedVerifier,
 				func(input []Operator) (Operator, error) {
 					return GetCastOperator(input[0], 0 /* inputIdx*/, 1 /* resultIdx */, c.fromTyp, c.toTyp)
 				})

pkg/sql/colexec/coalescer_test.go

@@ -51,11 +51,7 @@ func TestCoalescer(t *testing.T) {
 	}
 
 	for _, tc := range tcs {
-		colIndices := make([]int, len(tc.colTypes))
-		for i := 0; i < len(colIndices); i++ {
-			colIndices[i] = i
-		}
-		runTests(t, []tuples{tc.tuples}, tc.tuples, orderedVerifier, colIndices, func(input []Operator) (Operator, error) {
+		runTests(t, []tuples{tc.tuples}, tc.tuples, orderedVerifier, func(input []Operator) (Operator, error) {
 			return NewCoalescerOp(input[0], tc.colTypes), nil
 		})
 	}

pkg/sql/colexec/const_test.go

@@ -31,7 +31,7 @@ func TestConst(t *testing.T) {
 		},
 	}
 	for _, tc := range tcs {
-		runTestsWithTyps(t, []tuples{tc.tuples}, []coltypes.T{coltypes.Int64}, tc.expected, orderedVerifier, []int{0, 1},
+		runTestsWithTyps(t, []tuples{tc.tuples}, []coltypes.T{coltypes.Int64}, tc.expected, orderedVerifier,
 			func(input []Operator) (Operator, error) {
 				return NewConstOp(input[0], coltypes.Int64, int64(9), 1)
 			})
@@ -53,7 +53,7 @@ func TestConstNull(t *testing.T) {
 		},
 	}
 	for _, tc := range tcs {
-		runTestsWithTyps(t, []tuples{tc.tuples}, []coltypes.T{coltypes.Int64}, tc.expected, orderedVerifier, []int{0, 1},
+		runTestsWithTyps(t, []tuples{tc.tuples}, []coltypes.T{coltypes.Int64}, tc.expected, orderedVerifier,
 			func(input []Operator) (Operator, error) {
 				return NewConstNullOp(input[0], 1, coltypes.Int64), nil
 			})

pkg/sql/colexec/count_test.go

@@ -29,7 +29,7 @@ func TestCount(t *testing.T) {
 	for _, tc := range tcs {
 		// The tuples consisting of all nulls still count as separate rows, so if
 		// we replace all values with nulls, we should get the same output.
-		runTestsWithoutAllNullsInjection(t, []tuples{tc.tuples}, nil /* typs */, tc.expected, orderedVerifier, []int{0}, func(input []Operator) (Operator, error) {
+		runTestsWithoutAllNullsInjection(t, []tuples{tc.tuples}, nil /* typs */, tc.expected, orderedVerifier, func(input []Operator) (Operator, error) {
 			return NewCountOp(input[0]), nil
 		})
 	}

pkg/sql/colexec/deselector_test.go

@@ -62,7 +62,7 @@ func TestDeselector(t *testing.T) {
 	for _, tc := range tcs {
 		runTestsWithFixedSel(t, []tuples{tc.tuples}, tc.sel, func(t *testing.T, input []Operator) {
 			op := NewDeselectorOp(input[0], tc.colTypes)
-			out := newOpTestOutput(op, []int{0}, tc.expected)
+			out := newOpTestOutput(op, tc.expected)
 
 			if err := out.Verify(); err != nil {
 				t.Fatal(err)

pkg/sql/colexec/distinct_test.go

@@ -103,7 +103,7 @@ func TestSortedDistinct(t *testing.T) {
 	}
 
 	for _, tc := range tcs {
-		runTests(t, []tuples{tc.tuples}, tc.expected, orderedVerifier, []int{0, 1, 2, 3},
+		runTests(t, []tuples{tc.tuples}, tc.expected, orderedVerifier,
 			func(input []Operator) (Operator, error) {
 				return NewOrderedDistinct(input[0], tc.distinctCols, tc.colTypes)
 			})

pkg/sql/colexec/hashjoiner_test.go

@@ -721,12 +721,7 @@ func TestHashJoiner(t *testing.T) {
 
 		for _, buildDistinct := range buildFlags {
 			t.Run(fmt.Sprintf("buildDistinct=%v", buildDistinct), func(t *testing.T) {
-				cols := make([]int, len(tc.leftOutCols)+len(tc.rightOutCols))
-				for i := range cols {
-					cols[i] = i
-				}
-
-				runTests(t, inputs, tc.expectedTuples, unorderedVerifier, cols, func(sources []Operator) (Operator, error) {
+				runTests(t, inputs, tc.expectedTuples, unorderedVerifier, func(sources []Operator) (Operator, error) {
 					leftSource, rightSource := sources[0], sources[1]
 					return NewEqHashJoinerOp(
 						leftSource, rightSource,

pkg/sql/colexec/like_ops_test.go

@@ -87,7 +87,7 @@ func TestLikeOperators(t *testing.T) {
 		},
 	} {
 		runTests(
-			t, []tuples{tc.tups}, tc.expected, orderedVerifier, []int{0},
+			t, []tuples{tc.tups}, tc.expected, orderedVerifier,
 			func(input []Operator) (Operator, error) {
 				ctx := tree.MakeTestingEvalContext(cluster.MakeTestingClusterSettings())
 				return GetLikeOperator(&ctx, input[0], 0, tc.pattern, tc.negate)

pkg/sql/colexec/limit_test.go

@@ -58,7 +58,7 @@ func TestLimit(t *testing.T) {
 	for _, tc := range tcs {
 		// The tuples consisting of all nulls still count as separate rows, so if
 		// we replace all values with nulls, we should get the same output.
-		runTestsWithoutAllNullsInjection(t, []tuples{tc.tuples}, nil /* typs */, tc.expected, orderedVerifier, []int{0}, func(input []Operator) (Operator, error) {
+		runTestsWithoutAllNullsInjection(t, []tuples{tc.tuples}, nil /* typs */, tc.expected, orderedVerifier, func(input []Operator) (Operator, error) {
 			return NewLimitOp(input[0], tc.limit), nil
 		})
 	}

pkg/sql/colexec/mergejoiner_test.go

@@ -1458,7 +1458,7 @@ func TestMergeJoiner(t *testing.T) {
 			cols[i] = i
 		}
 		runner(t, []tuples{tc.leftTuples, tc.rightTuples}, nil /* typs */, tc.expected, mergeJoinVerifier,
-			cols, func(input []Operator) (Operator, error) {
+			func(input []Operator) (Operator, error) {
 				return NewMergeJoinOp(tc.joinType, input[0], input[1], tc.leftOutCols,
 					tc.rightOutCols, tc.leftTypes, tc.rightTypes, lOrderings, rOrderings,
 					nil /* filterConstructor */, false /* filterOnlyOnLeft */)

pkg/sql/colexec/offset_test.go

@@ -54,7 +54,7 @@ func TestOffset(t *testing.T) {
 	for _, tc := range tcs {
 		// The tuples consisting of all nulls still count as separate rows, so if
 		// we replace all values with nulls, we should get the same output.
-		runTestsWithoutAllNullsInjection(t, []tuples{tc.tuples}, nil /* typs */, tc.expected, unorderedVerifier, []int{0}, func(input []Operator) (Operator, error) {
+		runTestsWithoutAllNullsInjection(t, []tuples{tc.tuples}, nil /* typs */, tc.expected, unorderedVerifier, func(input []Operator) (Operator, error) {
 			return NewOffsetOp(input[0], tc.offset), nil
 		})
 	}

pkg/sql/colexec/orderedsynchronizer_test.go

@@ -136,12 +136,10 @@ func TestOrderedSync(t *testing.T) {
 	for _, tc := range testCases {
 		numCols := len(tc.sources[0][0])
 		columnTypes := make([]coltypes.T, numCols)
-		cols := make([]int, numCols)
 		for i := range columnTypes {
 			columnTypes[i] = coltypes.Int64
-			cols[i] = i
 		}
-		runTests(t, tc.sources, tc.expected, orderedVerifier, cols, func(inputs []Operator) (Operator, error) {
+		runTests(t, tc.sources, tc.expected, orderedVerifier, func(inputs []Operator) (Operator, error) {
 			return &OrderedSynchronizer{
 				inputs:      inputs,
 				ordering:    tc.ordering,
@@ -192,7 +190,7 @@ func TestOrderedSyncRandomInput(t *testing.T) {
 		columnTypes: []coltypes.T{coltypes.Int64},
 	}
 	op.Init()
-	out := newOpTestOutput(&op, []int{0}, expected)
+	out := newOpTestOutput(&op, expected)
 	if err := out.Verify(); err != nil {
 		t.Error(err)
 	}

pkg/sql/colexec/ordinality_test.go

@@ -42,8 +42,7 @@ func TestOrdinality(t *testing.T) {
 	}
 
 	for _, tc := range tcs {
-		numExpectedCols := len(tc.expected[0])
-		runTests(t, []tuples{tc.tuples}, tc.expected, orderedVerifier, []int{0, 1, 2}[:numExpectedCols],
+		runTests(t, []tuples{tc.tuples}, tc.expected, orderedVerifier,
 			func(input []Operator) (Operator, error) {
 				return NewOrdinalityOp(input[0]), nil
 			})

pkg/sql/colexec/projection_ops_test.go

@@ -25,7 +25,7 @@ import (
 
 func TestProjPlusInt64Int64ConstOp(t *testing.T) {
 	runTests(t, []tuples{{{1}, {2}, {nil}}}, tuples{{1, 2}, {2, 3}, {nil, nil}}, orderedVerifier,
-		[]int{0, 1}, func(input []Operator) (Operator, error) {
+		func(input []Operator) (Operator, error) {
 			return &projPlusInt64Int64ConstOp{
 				projConstOpBase: projConstOpBase{
 					OneInputNode: NewOneInputNode(input[0]),
@@ -39,7 +39,7 @@ func TestProjPlusInt64Int64ConstOp(t *testing.T) {
 
 func TestProjPlusInt64Int64Op(t *testing.T) {
 	runTests(t, []tuples{{{1, 2}, {3, 4}, {5, nil}}}, tuples{{1, 2, 3}, {3, 4, 7}, {5, nil, nil}},
-		orderedVerifier, []int{0, 1, 2},
+		orderedVerifier,
 		func(input []Operator) (Operator, error) {
 			return &projPlusInt64Int64Op{
 				projOpBase: projOpBase{

pkg/sql/colexec/routers_test.go

@@ -98,7 +98,7 @@ func TestRouterOutputAddBatch(t *testing.T) {
 				[]coltypes.T{coltypes.Int64}, unblockEventsChan, tc.blockedThreshold, tc.outputBatchSize,
 			)
 			in := newOpTestInput(tc.inputBatchSize, data, nil /* typs */)
-			out := newOpTestOutput(o, []int{0}, data[:len(tc.selection)])
+			out := newOpTestOutput(o, data[:len(tc.selection)])
 			in.Init()
 			for {
 				b := in.Next(ctx)
@@ -204,7 +204,7 @@ func TestRouterOutputNext(t *testing.T) {
 
 			// Should have data available, pushed by our reader goroutine.
 			batches := NewBatchBuffer()
-			out := newOpTestOutput(batches, []int{0}, tc.expected)
+			out := newOpTestOutput(batches, tc.expected)
 			for {
 				b := <-batchChan
 				batches.Add(b)
@@ -258,7 +258,7 @@ func TestRouterOutputNext(t *testing.T) {
 			[]coltypes.T{coltypes.Int64}, ch, blockThreshold, coldata.BatchSize,
 		)
 		in := newOpTestInput(smallBatchSize, data, nil /* typs */)
-		out := newOpTestOutput(o, []int{0}, expected)
+		out := newOpTestOutput(o, expected)
 		in.Init()
 
 		b := in.Next(ctx)
@@ -405,11 +405,7 @@ func TestRouterOutputRandom(t *testing.T) {
 
 			wg.Wait()
 
-			cols := make([]int, len(typs))
-			for i := range typs {
-				cols[i] = i
-			}
-			if err := newOpTestOutput(actual, cols, expected).Verify(); err != nil {
+			if err := newOpTestOutput(actual, expected).Verify(); err != nil {
 				t.Fatal(err)
 			}
 		})
@@ -625,7 +621,7 @@ func TestHashRouterOneOutput(t *testing.T) {
 		expected = append(expected, data[i])
 	}
 
-	o := newOpTestOutput(routerOutputs[0], []int{0}, expected)
+	o := newOpTestOutput(routerOutputs[0], expected)
 
 	var wg sync.WaitGroup
 	wg.Add(1)