singhpratech opened a new issue, #1305:
URL: https://github.com/apache/arrow-go/issues/1305
**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
<details>
<summary>and_kleene_chunksize_test.go (package compute_test; go test -run
TestAndKleeneChunkSize ./arrow/compute/)</summary>
```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)
}
}
}
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
