Skip to content

Commit

Permalink
exec: modify tests to catch bad selection vector access
Browse files Browse the repository at this point in the history
The runTests helper will now cause a panic if a vectorized operator
tries to access a part of the selection vector that is out of bounds.
This identified bugs in the projection operator.

Release note: None
  • Loading branch information
rafiss committed Aug 28, 2019
1 parent 53f64f9 commit fa2ae02
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 0 deletions.
3 changes: 3 additions & 0 deletions pkg/sql/exec/execgen/cmd/execgen/projection_ops_gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ func (p {{template "opRConstName" .}}) Next(ctx context.Context) coldata.Batch {
projVec := batch.ColVec(p.outputIdx)
projCol := projVec.{{.RetTyp}}()
if sel := batch.Selection(); sel != nil {
sel = sel[:n]
for _, i := range sel {
arg := {{.LTyp.Get "unsafe" "col" "int(i)"}}
{{(.Assign "projCol[i]" "arg" "p.constArg")}}
Expand Down Expand Up @@ -120,6 +121,7 @@ func (p {{template "opLConstName" .}}) Next(ctx context.Context) coldata.Batch {
projVec := batch.ColVec(p.outputIdx)
projCol := projVec.{{.RetTyp}}()
if sel := batch.Selection(); sel != nil {
sel = sel[:n]
for _, i := range sel {
arg := {{.RTyp.Get "unsafe" "col" "int(i)"}}
{{(.Assign "projCol[i]" "p.constArg" "arg")}}
Expand Down Expand Up @@ -175,6 +177,7 @@ func (p {{template "opName" .}}) Next(ctx context.Context) coldata.Batch {
col1 := vec1.{{.LTyp}}()
col2 := vec2.{{.RTyp}}()
if sel := batch.Selection(); sel != nil {
sel = sel[:n]
for _, i := range sel {
arg1 := {{.LTyp.Get "unsafe" "col1" "int(i)"}}
arg2 := {{.RTyp.Get "unsafe" "col2" "int(i)"}}
Expand Down
11 changes: 11 additions & 0 deletions pkg/sql/exec/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,9 @@ func (s *opTestInput) Next(context.Context) coldata.Batch {
}

if s.useSel {
for i := range s.selection {
s.selection[i] = uint16(i)
}
// We have populated s.selection vector with possibly more indices than we
// have actual tuples for, so some "default" tuples will be introduced but
// will not be selected due to the length of the batch being equal to the
Expand All @@ -285,6 +288,12 @@ func (s *opTestInput) Next(context.Context) coldata.Batch {
sort.Slice(s.selection[:batchSize], func(i, j int) bool {
return s.selection[i] < s.selection[j]
})
// Any unused elements in the selection vector are set to a value larger
// than the max batch size, so the test will panic if this part of the slice
// is accidentally accessed.
for i := range s.selection[batchSize:] {
s.selection[int(batchSize)+i] = coldata.BatchSize + 1
}

s.batch.SetSelection(true)
copy(s.batch.Selection(), s.selection)
Expand All @@ -304,6 +313,8 @@ func (s *opTestInput) Next(context.Context) coldata.Batch {
// reflection. This is slow, but acceptable for tests.
col := reflect.ValueOf(vec.Col())
for j := uint16(0); j < batchSize; j++ {
// If useSel is false, then the selection vector will contain
// [0, ..., batchSize] in ascending order.
outputIdx := s.selection[j]
if tups[j][i] == nil {
// Set garbage data in the value to make sure NULL gets handled
Expand Down

0 comments on commit fa2ae02

Please sign in to comment.