From 3ce6b32814070ca87d87f6be262f148727ba4bdb Mon Sep 17 00:00:00 2001 From: Jordan Lewis Date: Fri, 30 Aug 2019 00:48:33 -0400 Subject: [PATCH] exec: add random projection tests for all types This test randomly runs binary comparisons against all types with random data, verifying that the result of Datum.Compare matches the result of the exec projection. This found the bug with date infinity matching tracked in #40354, and immediately found a bug with the timestamp implementation in the previous commit, so I'm fairly sure it's a useful random test to have around. Release note: None --- pkg/sql/distsqlrun/materializer.go | 2 +- pkg/sql/exec/builtin_funcs.go | 2 +- pkg/sql/exec/projection_ops_test.go | 73 +++++++++++++++++++++++++++++ pkg/sql/exec/vec_elem_to_datum.go | 5 +- pkg/sql/row/cfetcher.go | 2 +- 5 files changed, 80 insertions(+), 4 deletions(-) diff --git a/pkg/sql/distsqlrun/materializer.go b/pkg/sql/distsqlrun/materializer.go index 0f2520c33015..d7cdb449bb1c 100644 --- a/pkg/sql/distsqlrun/materializer.go +++ b/pkg/sql/distsqlrun/materializer.go @@ -168,7 +168,7 @@ func (m *materializer) next() (sqlbase.EncDatumRow, *distsqlpb.ProducerMetadata) continue } } - m.row[colIdx].Datum = exec.PhysicalTypeColElemToDatum(col, rowIdx, m.da, typs[colIdx]) + m.row[colIdx].Datum = exec.PhysicalTypeColElemToDatum(col, rowIdx, m.da, &typs[colIdx]) } return m.ProcessRowHelper(m.row), nil } diff --git a/pkg/sql/exec/builtin_funcs.go b/pkg/sql/exec/builtin_funcs.go index 0670412433ce..adc8c97deaec 100644 --- a/pkg/sql/exec/builtin_funcs.go +++ b/pkg/sql/exec/builtin_funcs.go @@ -67,7 +67,7 @@ func (b *defaultBuiltinFuncOperator) Next(ctx context.Context) coldata.Batch { hasNulls = true b.row[j] = tree.DNull } else { - b.row[j] = PhysicalTypeColElemToDatum(col, rowIdx, b.da, b.columnTypes[b.argumentCols[j]]) + b.row[j] = PhysicalTypeColElemToDatum(col, rowIdx, b.da, &b.columnTypes[b.argumentCols[j]]) } } diff --git a/pkg/sql/exec/projection_ops_test.go b/pkg/sql/exec/projection_ops_test.go index e18e7cafb812..b161b11f111a 100644 --- a/pkg/sql/exec/projection_ops_test.go +++ b/pkg/sql/exec/projection_ops_test.go @@ -19,8 +19,13 @@ import ( "github.com/cockroachdb/cockroach/pkg/col/coldata" "github.com/cockroachdb/cockroach/pkg/col/coltypes" + "github.com/cockroachdb/cockroach/pkg/settings/cluster" + "github.com/cockroachdb/cockroach/pkg/sql/exec/typeconv" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" + "github.com/cockroachdb/cockroach/pkg/sql/sqlbase" "github.com/cockroachdb/cockroach/pkg/sql/types" + "github.com/cockroachdb/cockroach/pkg/util/randutil" + "github.com/stretchr/testify/assert" ) func TestProjPlusInt64Int64ConstOp(t *testing.T) { @@ -142,6 +147,74 @@ func TestGetProjectionConstMixedTypeOperator(t *testing.T) { } } +func TestRandomComparisons(t *testing.T) { + rng, _ := randutil.NewPseudoRand() + evalCtx := tree.NewTestingEvalContext(cluster.MakeTestingClusterSettings()) + ctx := evalCtx.Ctx() + expected := make([]bool, 2048) + var da sqlbase.DatumAlloc + lDatums := make([]tree.Datum, 2048) + rDatums := make([]tree.Datum, 2048) + for _, ct := range types.Scalar { + if ct.Family() == types.DateFamily { + // TODO(jordan): #40354 tracks failure to compare infinite dates. + continue + } + typ := typeconv.FromColumnType(ct) + if typ == coltypes.Unhandled { + continue + } + typs := []coltypes.T{typ, typ, coltypes.Bool} + b := coldata.NewMemBatchWithSize(typs, 2048) + lVec := b.ColVec(0) + rVec := b.ColVec(1) + ret := b.ColVec(2) + RandomVec(rng, typ, lVec, 2048, 0) + RandomVec(rng, typ, rVec, 2048, 0) + for i := range lDatums { + lDatums[i] = PhysicalTypeColElemToDatum(lVec, uint16(i), da, ct) + rDatums[i] = PhysicalTypeColElemToDatum(rVec, uint16(i), da, ct) + } + for _, cmpOp := range []tree.ComparisonOperator{tree.EQ, tree.NE, tree.LT, tree.LE, tree.GT, tree.GE} { + for i := range lDatums { + cmp := lDatums[i].Compare(evalCtx, rDatums[i]) + var b bool + switch cmpOp { + case tree.EQ: + b = cmp == 0 + case tree.NE: + b = cmp != 0 + case tree.LT: + b = cmp < 0 + case tree.LE: + b = cmp <= 0 + case tree.GT: + b = cmp > 0 + case tree.GE: + b = cmp >= 0 + } + expected[i] = b + } + input := newChunkingBatchSource(typs, []coldata.Vec{lVec, rVec, ret}, 2048) + op, err := GetProjectionOperator(ct, ct, cmpOp, input, 0, 1, 2) + if err != nil { + t.Fatal(err) + } + op.Init() + var idx uint16 + for batch := op.Next(ctx); batch.Length() > 0; batch = op.Next(ctx) { + for i := uint16(0); i < batch.Length(); i++ { + absIdx := idx + i + assert.Equal(t, expected[absIdx], batch.ColVec(2).Bool()[i], + "expected %s %s %s (%s[%d]) to be %t found %t", lDatums[absIdx], cmpOp, rDatums[absIdx], ct, absIdx, + expected[absIdx], ret.Bool()[i]) + } + idx += batch.Length() + } + } + } +} + func TestGetProjectionOperator(t *testing.T) { ct := types.Int2 binOp := tree.Mult diff --git a/pkg/sql/exec/vec_elem_to_datum.go b/pkg/sql/exec/vec_elem_to_datum.go index 5a270c8852cb..da2260081508 100644 --- a/pkg/sql/exec/vec_elem_to_datum.go +++ b/pkg/sql/exec/vec_elem_to_datum.go @@ -12,6 +12,7 @@ package exec import ( "fmt" + "time" "unsafe" "github.com/cockroachdb/cockroach/pkg/col/coldata" @@ -25,7 +26,7 @@ import ( // PhysicalTypeColElemToDatum converts an element in a colvec to a datum of semtype ct. func PhysicalTypeColElemToDatum( - col coldata.Vec, rowIdx uint16, da sqlbase.DatumAlloc, ct types.T, + col coldata.Vec, rowIdx uint16, da sqlbase.DatumAlloc, ct *types.T, ) tree.Datum { switch ct.Family() { case types.BoolFamily: @@ -60,6 +61,8 @@ func PhysicalTypeColElemToDatum( return da.NewDBytes(tree.DBytes(col.Bytes().Get(int(rowIdx)))) case types.OidFamily: return da.NewDOid(tree.MakeDOid(tree.DInt(col.Int64()[rowIdx]))) + case types.TimestampFamily: + return tree.MakeDTimestamp(col.Timestamp()[rowIdx], time.Microsecond) default: execerror.VectorizedInternalPanic(fmt.Sprintf("Unsupported column type %s", ct.String())) // This code is unreachable, but the compiler cannot infer that. diff --git a/pkg/sql/row/cfetcher.go b/pkg/sql/row/cfetcher.go index 4e6df34359a1..8b0a13999d80 100644 --- a/pkg/sql/row/cfetcher.go +++ b/pkg/sql/row/cfetcher.go @@ -789,7 +789,7 @@ func (rf *CFetcher) getDatumAt(colIdx int, rowIdx uint16, typ types.T) tree.Datu if rf.machine.colvecs[colIdx].Nulls().NullAt(rowIdx) { return tree.DNull } - return exec.PhysicalTypeColElemToDatum(rf.machine.colvecs[colIdx], rowIdx, rf.table.da, typ) + return exec.PhysicalTypeColElemToDatum(rf.machine.colvecs[colIdx], rowIdx, rf.table.da, &typ) } // processValue processes the state machine's current value component, setting