From 4eba8720e661a8abfe9f1d4f10f7e6092f3547cc Mon Sep 17 00:00:00 2001 From: Yahor Yuzefovich Date: Fri, 13 Sep 2019 21:11:07 -0700 Subject: [PATCH] colexec: remove cols argument from runTests harness 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 --- pkg/sql/colexec/aggregator_test.go | 14 +++----- pkg/sql/colexec/and_test.go | 25 +++++++------- pkg/sql/colexec/bool_vec_to_sel_test.go | 2 +- pkg/sql/colexec/builtin_funcs_test.go | 6 ++-- pkg/sql/colexec/cast_test.go | 4 +-- pkg/sql/colexec/coalescer_test.go | 6 +--- pkg/sql/colexec/const_test.go | 4 +-- pkg/sql/colexec/count_test.go | 2 +- pkg/sql/colexec/deselector_test.go | 2 +- pkg/sql/colexec/distinct_test.go | 2 +- pkg/sql/colexec/hashjoiner_test.go | 7 +--- pkg/sql/colexec/like_ops_test.go | 2 +- pkg/sql/colexec/limit_test.go | 2 +- pkg/sql/colexec/mergejoiner_test.go | 2 +- pkg/sql/colexec/offset_test.go | 2 +- pkg/sql/colexec/orderedsynchronizer_test.go | 6 ++-- pkg/sql/colexec/ordinality_test.go | 3 +- pkg/sql/colexec/projection_ops_test.go | 4 +-- pkg/sql/colexec/routers_test.go | 14 +++----- pkg/sql/colexec/select_in_test.go | 18 +++++----- pkg/sql/colexec/selection_ops_test.go | 4 +-- pkg/sql/colexec/simple_project_test.go | 4 +-- pkg/sql/colexec/sort_chunks_test.go | 12 ++----- pkg/sql/colexec/sort_test.go | 12 ++----- pkg/sql/colexec/sorttopk_test.go | 6 +--- pkg/sql/colexec/utils_test.go | 38 +++++++++------------ 26 files changed, 77 insertions(+), 126 deletions(-) diff --git a/pkg/sql/colexec/aggregator_test.go b/pkg/sql/colexec/aggregator_test.go index 6196d27e5869..022f5b9f0165 100644 --- a/pkg/sql/colexec/aggregator_test.go +++ b/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 */) }) } diff --git a/pkg/sql/colexec/and_test.go b/pkg/sql/colexec/and_test.go index a242a2e1ca2e..65fb9f67b6ec 100644 --- a/pkg/sql/colexec/and_test.go +++ b/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], diff --git a/pkg/sql/colexec/bool_vec_to_sel_test.go b/pkg/sql/colexec/bool_vec_to_sel_test.go index afbf9139ecd1..9170c850beac 100644 --- a/pkg/sql/colexec/bool_vec_to_sel_test.go +++ b/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 }) } diff --git a/pkg/sql/colexec/builtin_funcs_test.go b/pkg/sql/colexec/builtin_funcs_test.go index 04869137ac22..103daa33dc04 100644 --- a/pkg/sql/colexec/builtin_funcs_test.go +++ b/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 { diff --git a/pkg/sql/colexec/cast_test.go b/pkg/sql/colexec/cast_test.go index c20367a142e8..5e0d3ae67662 100644 --- a/pkg/sql/colexec/cast_test.go +++ b/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) }) diff --git a/pkg/sql/colexec/coalescer_test.go b/pkg/sql/colexec/coalescer_test.go index 712e83db24ee..75d192428475 100644 --- a/pkg/sql/colexec/coalescer_test.go +++ b/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 }) } diff --git a/pkg/sql/colexec/const_test.go b/pkg/sql/colexec/const_test.go index b2a870e315ca..09fbd7a350d1 100644 --- a/pkg/sql/colexec/const_test.go +++ b/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 }) diff --git a/pkg/sql/colexec/count_test.go b/pkg/sql/colexec/count_test.go index 0e157dc8bf70..baa25323dc9a 100644 --- a/pkg/sql/colexec/count_test.go +++ b/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 }) } diff --git a/pkg/sql/colexec/deselector_test.go b/pkg/sql/colexec/deselector_test.go index f8af7ccd1d2e..6a528f04843a 100644 --- a/pkg/sql/colexec/deselector_test.go +++ b/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) diff --git a/pkg/sql/colexec/distinct_test.go b/pkg/sql/colexec/distinct_test.go index ad52b53634c5..daa6f5bbab21 100644 --- a/pkg/sql/colexec/distinct_test.go +++ b/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) }) diff --git a/pkg/sql/colexec/hashjoiner_test.go b/pkg/sql/colexec/hashjoiner_test.go index f530af8c9ab3..368b77351472 100644 --- a/pkg/sql/colexec/hashjoiner_test.go +++ b/pkg/sql/colexec/hashjoiner_test.go @@ -720,12 +720,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, diff --git a/pkg/sql/colexec/like_ops_test.go b/pkg/sql/colexec/like_ops_test.go index 0aa2eaec4122..5303b7bb3b95 100644 --- a/pkg/sql/colexec/like_ops_test.go +++ b/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) diff --git a/pkg/sql/colexec/limit_test.go b/pkg/sql/colexec/limit_test.go index ea42f9abde68..677fd4d3aca9 100644 --- a/pkg/sql/colexec/limit_test.go +++ b/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 }) } diff --git a/pkg/sql/colexec/mergejoiner_test.go b/pkg/sql/colexec/mergejoiner_test.go index 2dfcbfb50be5..0c5e447c7f60 100644 --- a/pkg/sql/colexec/mergejoiner_test.go +++ b/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 */) diff --git a/pkg/sql/colexec/offset_test.go b/pkg/sql/colexec/offset_test.go index 07af1d244b46..5dd6f866c425 100644 --- a/pkg/sql/colexec/offset_test.go +++ b/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 }) } diff --git a/pkg/sql/colexec/orderedsynchronizer_test.go b/pkg/sql/colexec/orderedsynchronizer_test.go index c7ba0ead5d26..0b66ab8a68de 100644 --- a/pkg/sql/colexec/orderedsynchronizer_test.go +++ b/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) } diff --git a/pkg/sql/colexec/ordinality_test.go b/pkg/sql/colexec/ordinality_test.go index 34094d98a817..9536b3709b4c 100644 --- a/pkg/sql/colexec/ordinality_test.go +++ b/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 }) diff --git a/pkg/sql/colexec/projection_ops_test.go b/pkg/sql/colexec/projection_ops_test.go index 4f304c053017..0a54ffbfaa01 100644 --- a/pkg/sql/colexec/projection_ops_test.go +++ b/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{ diff --git a/pkg/sql/colexec/routers_test.go b/pkg/sql/colexec/routers_test.go index d747f657115b..c3345fa18917 100644 --- a/pkg/sql/colexec/routers_test.go +++ b/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) diff --git a/pkg/sql/colexec/select_in_test.go b/pkg/sql/colexec/select_in_test.go index cb1e2d9d8739..9bd8b332ffb2 100644 --- a/pkg/sql/colexec/select_in_test.go +++ b/pkg/sql/colexec/select_in_test.go @@ -76,7 +76,7 @@ func TestSelectInInt64(t *testing.T) { return &op, nil } if !c.hasNulls || !c.negate { - runTests(t, []tuples{c.inputTuples}, c.outputTuples, orderedVerifier, []int{0}, opConstructor) + runTests(t, []tuples{c.inputTuples}, c.outputTuples, orderedVerifier, opConstructor) } else { // When the input tuples already have nulls and we have NOT IN // operator, then the nulls injection might not change the output. For @@ -84,7 +84,7 @@ func TestSelectInInt64(t *testing.T) { // output of length 0; similarly, we will get the same zero-length // output for the corresponding nulls injection test case // "1 NOT IN (NULL, NULL, NULL)". - runTestsWithoutAllNullsInjection(t, []tuples{c.inputTuples}, nil /* typs */, c.outputTuples, orderedVerifier, []int{0}, opConstructor) + runTestsWithoutAllNullsInjection(t, []tuples{c.inputTuples}, nil /* typs */, c.outputTuples, orderedVerifier, opConstructor) } }) } @@ -159,7 +159,7 @@ func TestProjectInInt64(t *testing.T) { { desc: "Simple in test", inputTuples: tuples{{0}, {1}}, - outputTuples: tuples{{true}, {true}}, + outputTuples: tuples{{0, true}, {1, true}}, filterRow: []int64{0, 1}, hasNulls: false, negate: false, @@ -167,7 +167,7 @@ func TestProjectInInt64(t *testing.T) { { desc: "Simple not in test", inputTuples: tuples{{2}}, - outputTuples: tuples{{true}}, + outputTuples: tuples{{2, true}}, filterRow: []int64{0, 1}, hasNulls: false, negate: true, @@ -175,7 +175,7 @@ func TestProjectInInt64(t *testing.T) { { desc: "In test with NULLs", inputTuples: tuples{{1}, {2}, {nil}}, - outputTuples: tuples{{true}, {nil}, {nil}}, + outputTuples: tuples{{1, true}, {2, nil}, {nil, nil}}, filterRow: []int64{1}, hasNulls: true, negate: false, @@ -183,7 +183,7 @@ func TestProjectInInt64(t *testing.T) { { desc: "Not in test with NULLs", inputTuples: tuples{{1}, {2}, {nil}}, - outputTuples: tuples{{false}, {nil}, {nil}}, + outputTuples: tuples{{1, false}, {2, nil}, {nil, nil}}, filterRow: []int64{1}, hasNulls: true, negate: true, @@ -191,7 +191,7 @@ func TestProjectInInt64(t *testing.T) { { desc: "Not in test with NULLs and no nulls in filter", inputTuples: tuples{{1}, {2}, {nil}}, - outputTuples: tuples{{false}, {true}, {nil}}, + outputTuples: tuples{{1, false}, {2, true}, {nil, nil}}, filterRow: []int64{1}, hasNulls: false, negate: true, @@ -199,7 +199,7 @@ func TestProjectInInt64(t *testing.T) { { desc: "Test with false values", inputTuples: tuples{{1}, {2}}, - outputTuples: tuples{{false}, {false}}, + outputTuples: tuples{{1, false}, {2, false}}, filterRow: []int64{3}, hasNulls: false, negate: false, @@ -208,7 +208,7 @@ func TestProjectInInt64(t *testing.T) { for _, c := range testCases { t.Run(c.desc, func(t *testing.T) { - runTests(t, []tuples{c.inputTuples}, c.outputTuples, orderedVerifier, []int{1}, + runTests(t, []tuples{c.inputTuples}, c.outputTuples, orderedVerifier, func(input []Operator) (Operator, error) { op := projectInOpInt64{ OneInputNode: NewOneInputNode(input[0]), diff --git a/pkg/sql/colexec/selection_ops_test.go b/pkg/sql/colexec/selection_ops_test.go index ef4f00543d2d..0fa639f0efa5 100644 --- a/pkg/sql/colexec/selection_ops_test.go +++ b/pkg/sql/colexec/selection_ops_test.go @@ -31,7 +31,7 @@ const ( func TestSelLTInt64Int64ConstOp(t *testing.T) { tups := tuples{{0}, {1}, {2}, {nil}} - runTests(t, []tuples{tups}, tuples{{0}, {1}}, orderedVerifier, []int{0}, func(input []Operator) (Operator, error) { + runTests(t, []tuples{tups}, tuples{{0}, {1}}, orderedVerifier, func(input []Operator) (Operator, error) { return &selLTInt64Int64ConstOp{ selConstOpBase: selConstOpBase{ OneInputNode: NewOneInputNode(input[0]), @@ -52,7 +52,7 @@ func TestSelLTInt64Int64(t *testing.T) { {-1, nil}, {nil, nil}, } - runTests(t, []tuples{tups}, tuples{{0, 1}}, orderedVerifier, []int{0, 1}, func(input []Operator) (Operator, error) { + runTests(t, []tuples{tups}, tuples{{0, 1}}, orderedVerifier, func(input []Operator) (Operator, error) { return &selLTInt64Int64Op{ selOpBase: selOpBase{ OneInputNode: NewOneInputNode(input[0]), diff --git a/pkg/sql/colexec/simple_project_test.go b/pkg/sql/colexec/simple_project_test.go index 961450e3d14b..7f6f9e1e9385 100644 --- a/pkg/sql/colexec/simple_project_test.go +++ b/pkg/sql/colexec/simple_project_test.go @@ -59,14 +59,14 @@ func TestSimpleProjectOp(t *testing.T) { }, } for _, tc := range tcs { - runTests(t, []tuples{tc.tuples}, tc.expected, orderedVerifier, []int{0, 1}, func(input []Operator) (Operator, error) { + runTests(t, []tuples{tc.tuples}, tc.expected, orderedVerifier, func(input []Operator) (Operator, error) { return NewSimpleProjectOp(input[0], len(tc.tuples[0]), tc.colsToKeep), nil }) } // Empty projection. The all nulls injection test case will also return // nothing. - runTestsWithoutAllNullsInjection(t, []tuples{{{1, 2, 3}, {1, 2, 3}}}, nil /* typs */, tuples{{}, {}}, orderedVerifier, []int{}, + runTestsWithoutAllNullsInjection(t, []tuples{{{1, 2, 3}, {1, 2, 3}}}, nil /* typs */, tuples{{}, {}}, orderedVerifier, func(input []Operator) (Operator, error) { return NewSimpleProjectOp(input[0], 3 /* numInputCols */, nil), nil }) diff --git a/pkg/sql/colexec/sort_chunks_test.go b/pkg/sql/colexec/sort_chunks_test.go index cc12b24b4739..49c2302fb81b 100644 --- a/pkg/sql/colexec/sort_chunks_test.go +++ b/pkg/sql/colexec/sort_chunks_test.go @@ -192,11 +192,7 @@ func TestSortChunks(t *testing.T) { }, } for _, tc := range tcs { - cols := make([]int, len(tc.typ)) - for i := range cols { - cols[i] = i - } - runTests(t, []tuples{tc.tuples}, tc.expected, orderedVerifier, cols, func(input []Operator) (Operator, error) { + runTests(t, []tuples{tc.tuples}, tc.expected, orderedVerifier, func(input []Operator) (Operator, error) { return NewSortChunks(input[0], tc.typ, tc.ordCols, tc.matchLen) }) } @@ -236,11 +232,7 @@ func TestSortChunksRandomized(t *testing.T) { copy(expected, tups) sort.Slice(expected, less(expected, ordCols)) - cols := make([]int, nCols) - for i := range cols { - cols[i] = i - } - runTests(t, []tuples{sortedTups}, expected, orderedVerifier, cols, func(input []Operator) (Operator, error) { + runTests(t, []tuples{sortedTups}, expected, orderedVerifier, func(input []Operator) (Operator, error) { return NewSortChunks(input[0], typs[:nCols], ordCols, matchLen) }) } diff --git a/pkg/sql/colexec/sort_test.go b/pkg/sql/colexec/sort_test.go index a02736b8ad07..15e2b6eaf07f 100644 --- a/pkg/sql/colexec/sort_test.go +++ b/pkg/sql/colexec/sort_test.go @@ -138,11 +138,7 @@ func TestSort(t *testing.T) { }, } for _, tc := range tcs { - cols := make([]int, len(tc.typ)) - for i := range cols { - cols[i] = i - } - runTests(t, []tuples{tc.tuples}, tc.expected, orderedVerifier, cols, func(input []Operator) (Operator, error) { + runTests(t, []tuples{tc.tuples}, tc.expected, orderedVerifier, func(input []Operator) (Operator, error) { return NewSorter(input[0], tc.typ, tc.ordCols) }) } @@ -188,11 +184,7 @@ func TestSortRandomized(t *testing.T) { expected = expected[:k] } - cols := make([]int, nCols) - for i := range cols { - cols[i] = i - } - runTests(t, []tuples{tups}, expected, orderedVerifier, cols, func(input []Operator) (Operator, error) { + runTests(t, []tuples{tups}, expected, orderedVerifier, func(input []Operator) (Operator, error) { if topK { return NewTopKSorter(input[0], typs[:nCols], ordCols, k), nil } diff --git a/pkg/sql/colexec/sorttopk_test.go b/pkg/sql/colexec/sorttopk_test.go index 3800124d2edf..eb68ef663c31 100644 --- a/pkg/sql/colexec/sorttopk_test.go +++ b/pkg/sql/colexec/sorttopk_test.go @@ -67,11 +67,7 @@ func TestTopKSorter(t *testing.T) { } for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { - cols := make([]int, len(tc.typ)) - for i := range cols { - cols[i] = i - } - runTests(t, []tuples{tc.tuples}, tc.expected, orderedVerifier, cols, func(input []Operator) (Operator, error) { + runTests(t, []tuples{tc.tuples}, tc.expected, orderedVerifier, func(input []Operator) (Operator, error) { return NewTopKSorter(input[0], tc.typ, tc.ordCols, tc.k), nil }) }) diff --git a/pkg/sql/colexec/utils_test.go b/pkg/sql/colexec/utils_test.go index bc849ea526a3..39724b52a45c 100644 --- a/pkg/sql/colexec/utils_test.go +++ b/pkg/sql/colexec/utils_test.go @@ -76,7 +76,7 @@ func maybeHasNulls(b coldata.Batch) bool { return false } -type testRunner func(*testing.T, []tuples, []coltypes.T, tuples, verifier, []int, func([]Operator) (Operator, error)) +type testRunner func(*testing.T, []tuples, []coltypes.T, tuples, verifier, func([]Operator) (Operator, error)) // variableOutputBatchSizeInitializer is implemented by operators that can be // initialized with variable output size batches. This allows runTests to @@ -96,10 +96,9 @@ func runTests( tups []tuples, expected tuples, verifier verifier, - cols []int, constructor func(inputs []Operator) (Operator, error), ) { - runTestsWithTyps(t, tups, nil /* typs */, expected, verifier, cols, constructor) + runTestsWithTyps(t, tups, nil /* typs */, expected, verifier, constructor) } // runTestsWithTyps is the same as runTests with an ability to specify the @@ -110,10 +109,9 @@ func runTestsWithTyps( typs []coltypes.T, expected tuples, verifier verifier, - cols []int, constructor func(inputs []Operator) (Operator, error), ) { - runTestsWithoutAllNullsInjection(t, tups, typs, expected, verifier, cols, constructor) + runTestsWithoutAllNullsInjection(t, tups, typs, expected, verifier, constructor) t.Run("allNullsInjection", func(t *testing.T) { // This test replaces all values in the input tuples with nulls and ensures @@ -162,8 +160,8 @@ func runTestsWithTyps( var originalTuples, tuplesWithNulls tuples for i := uint16(0); i < originalBatch.Length(); i++ { // We checked that the batches have the same length. - originalTuples = append(originalTuples, getTupleFromBatch(originalBatch, cols, i)) - tuplesWithNulls = append(tuplesWithNulls, getTupleFromBatch(batchWithNulls, cols, i)) + originalTuples = append(originalTuples, getTupleFromBatch(originalBatch, i)) + tuplesWithNulls = append(tuplesWithNulls, getTupleFromBatch(batchWithNulls, i)) } if err := assertTuplesSetsEqual(originalTuples, tuplesWithNulls); err != nil { // err is non-nil which means that the batches are different. @@ -194,7 +192,6 @@ func runTestsWithoutAllNullsInjection( typs []coltypes.T, expected tuples, verifier verifier, - cols []int, constructor func(inputs []Operator) (Operator, error), ) { runTestsWithFn(t, tups, typs, func(t *testing.T, inputs []Operator) { @@ -202,7 +199,7 @@ func runTestsWithoutAllNullsInjection( if err != nil { t.Fatal(err) } - out := newOpTestOutput(op, cols, expected) + out := newOpTestOutput(op, expected) if err := verifier(out); err != nil { t.Fatal(err) } @@ -684,7 +681,6 @@ func (s *opFixedSelTestInput) Next(context.Context) coldata.Batch { // match some expected output tuples. type opTestOutput struct { OneInputNode - cols []int expected tuples curIdx uint16 @@ -692,30 +688,28 @@ type opTestOutput struct { } // newOpTestOutput returns a new opTestOutput, initialized with the given input -// to verify on the given column indices that the output is exactly equal to -// the expected tuples. -func newOpTestOutput(input Operator, cols []int, expected tuples) *opTestOutput { +// to verify that the output is exactly equal to the expected tuples. +func newOpTestOutput(input Operator, expected tuples) *opTestOutput { input.Init() return &opTestOutput{ OneInputNode: NewOneInputNode(input), - cols: cols, expected: expected, } } // getTupleFromBatch is a helper function that extracts a tuple at index -// tupleIdx from batch using only columns in cols. -func getTupleFromBatch(batch coldata.Batch, cols []int, tupleIdx uint16) tuple { - ret := make(tuple, len(cols)) +// tupleIdx from batch. +func getTupleFromBatch(batch coldata.Batch, tupleIdx uint16) tuple { + ret := make(tuple, batch.Width()) out := reflect.ValueOf(ret) if sel := batch.Selection(); sel != nil { tupleIdx = sel[tupleIdx] } - for outIdx, colIdx := range cols { + for colIdx := range ret { vec := batch.ColVec(colIdx) if vec.Nulls().NullAt(tupleIdx) { - ret[outIdx] = nil + ret[colIdx] = nil } else { var val reflect.Value if colBytes, ok := vec.Col().(*coldata.Bytes); ok { @@ -728,7 +722,7 @@ func getTupleFromBatch(batch coldata.Batch, cols []int, tupleIdx uint16) tuple { } else { val = reflect.ValueOf(vec.Col()).Index(int(tupleIdx)) } - out.Index(outIdx).Set(val) + out.Index(colIdx).Set(val) } } return ret @@ -743,7 +737,7 @@ func (r *opTestOutput) next(ctx context.Context) tuple { } r.curIdx = 0 } - ret := getTupleFromBatch(r.batch, r.cols, r.curIdx) + ret := getTupleFromBatch(r.batch, r.curIdx) r.curIdx++ return ret } @@ -998,7 +992,7 @@ func TestOpTestInputOutput(t *testing.T) { }, } runTestsWithFn(t, inputs, nil /* typs */, func(t *testing.T, sources []Operator) { - out := newOpTestOutput(sources[0], []int{0, 1, 2}, inputs[0]) + out := newOpTestOutput(sources[0], inputs[0]) if err := out.Verify(); err != nil { t.Fatal(err)