[Compute] and_kleene / or_kleene return wrong values when ExecCtx.ChunkSize is smaller than the input: ArraySpan.SetSlice carries a stale cached null count into the next slice
- Dominant language
- Assembly
- Stars
- 404
- Forks
- 145
- Avg merge
- 2d 4h
- Merged PRs (30d)
- 87
Description
**Describe the bug**
With `ExecCtx.ChunkSize` set below the input length, `and_kleene` and `or_kleene` return `false`
where the answer is null. The result depends on the chunk size, which it must not.
```go
// left: [true, null, true, null, false, true, null, true]
// right: [null, true, true, false, null, null, true, true]
ectx := compute.DefaultExecCtx()
ectx.ChunkSize = 1
ctx := compute.SetExecCtx(context.Background(), ectx)
out, _ := compute.CallFunction(ctx, "and_kleene", nil, leftDatum, rightDatum)
```
| ChunkSize | and_kleene result |
|---|---|
| default | `[null null true false false null null true]` |
| 1 | `[null false true false false null false true]` |
| 2 | `[null null true false false false false true]` |
| 3 and above | correct |
arrow-go main (281776d) and v18.7.0, whose `SetSlice` is the same code; Go 1.27.1, macOS 26.6 arm64. Full test in the attached file; the
CheckedAllocator in it reports no leak, this is a values bug only.
**Cause**
`iterateExecSpans` reuses one `ArraySpan` per argument and advances it with `SetSlice`
(executor.go:902). `ArraySpan.SetSlice` (exec/span.go:211-230) reuses the cached `Nulls` for the
new slice when it is `0` ("no nulls") or equal to the old `Len` ("all null"), and only otherwise
resets it to `UnknownNullCount`. That is right only while `Nulls` describes the whole array. The
Kleene kernels call `UpdateNullCount()` on the span they are given (scalar_boolean.go:49, 54, 97,
121, 131, 174, 198), which stores the *current slice's* count into the shared span; on the next
`SetSlice` a slice that happened to be all valid makes every following slice "no nulls", and a
slice that was all null makes every following slice "all null". Any kernel that calls
`UpdateNullCount()` on its input span is exposed; the Kleene kernels are the ones on main that do.
**Expected behavior**
Results independent of `ChunkSize`. The C++ `ArraySpan::SetSlice` never carries a cached count
when a validity bitmap is present; the same rule here fixes it:
```go
if a.Type.ID() != arrow.NULL {
if a.Nulls != 0 || len(a.Buffers[0].Buf) != 0 {
a.Nulls = array.UnknownNullCount
}
} else {
a.Nulls = length
}
```
With that change the attached test passes for every chunk size and `go test ./arrow/compute/...`
still passes. I can open the PR with the fix and the test if that is welcome.
**Component(s)**
Go, Compute
and_kleene_chunksize_test.go (package compute_test; go test -run TestAndKleeneChunkSize ./arrow/compute/)
```go
package compute_test
import (
"context"
"testing"
"github.com/apache/arrow-go/v18/arrow/array"
"github.com/apache/arrow-go/v18/arrow/compute"
"github.com/apache/arrow-go/v18/arrow/memory"
)
// and_kleene over two boolean arrays with nulls, executed with different
// ExecCtx.ChunkSize values. The result must not depend on ChunkSize.
func TestAndKleeneChunkSize(t *testing.T) {
mem := memory.NewCheckedAllocator(memory.DefaultAllocator)
defer mem.AssertSize(t, 0)
// left: [true, null, true, null, false, true, null, true]
// right: [null, true, true, false, null, null, true, true]
lb := array.NewBooleanBuilder(mem)
lb.AppendValues([]bool{true, false, true, false, false, true, false, true}, []bool{true, false, true, false, true, true, false, true})
left := lb.NewArray()
lb.Release()
defer left.Release()
rb := array.NewBooleanBuilder(mem)
rb.AppendValues([]bool{false, true, true, false, false, false, true, true}, []bool{false, true, true, true, false, false, true, true})
right := rb.NewArray()
rb.Release()
defer right.Release()
run := func(chunk int64) string {
ectx := compute.DefaultExecCtx()
ectx.ChunkSize = chunk
ctx := compute.SetExecCtx(compute.WithAllocator(context.Background(), mem), ectx)
out, err := compute.CallFunction(ctx, "and_kleene", nil, &compute.ArrayDatum{Value: left.Data()}, &compute.ArrayDatum{Value: right.Data()})
if err != nil {
t.Fatal(err)
}
defer out.Release()
arr := out.(*compute.ArrayDatum).MakeArray()
defer arr.Release()
return arr.String()
}
ref := run(compute.DefaultMaxChunkSize)
t.Logf("ChunkSize default: %s", ref)
for _, n := range []int64{1, 2, 3, 4, 8} {
got := run(n)
t.Logf("ChunkSize %d: %s", n, got)
if got != ref {
t.Errorf("ChunkSize %d: got %s, want %s", n, got, ref)
}
}
}
```
Contributor guide
Research direction
Start with ArraySpan.SetSlice in exec/span.go and inspect how iterateExecSpans in executor.go reuses spans; then review the Kleene callers in scalar_boolean.go. Add the supplied and_kleene chunk-size regression test and run go test -run TestAndKleeneChunkSize ./arrow/compute/ plus go test ./arrow/compute/; done means results match across all chunk sizes without allocator leaks.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- data
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 90/100