zeroshade opened a new pull request, #1408:
URL: https://github.com/apache/arrow-go/pull/1408
### Rationale for this change
`TestConcatRunEndEncodedInt64FinalRunEndClamp` panics on 386 (`GOARCH=386 go
test ./arrow/array -run TestConcatRunEndEncodedInt64FinalRunEndClamp`):
```
panic: arrow/array: index out of range
arrow/array/data.go:245 (NewSliceData)
arrow/array/concat_test.go:1231
```
The test builds its input with `RunEndEncodedBuilder.Append(math.MaxInt64)`,
which makes an array whose length is `math.MaxInt64`. Array lengths are `int`,
so on 32-bit targets the length wraps negative and `array.NewSlice(big, 0, 1)`
panics while *building the input*: the test never reaches `Concatenate`. That
is exactly the platform it was written for, since #1219's original bug (an
`int(e)` conversion of the run end) only overflowed when `int` is 32 bits.
Two related tests, `TestConcatAlmostOverflowRunEndEncoding/int64` and
`TestConcatOverflowRunEndEncoding/int64`, have the same problem: they build
inputs whose logical lengths are near 2^62, which cannot exist with 32-bit
`int`.
These panics are not visible in CI, which only cross-builds for 386.
### What changes are included in this PR?
Test-only changes in `arrow/array/concat_test.go`:
- `TestConcatRunEndEncodedInt64FinalRunEndClamp` builds the one-element
input directly with `NewRunEndEncodedArray`: run ends `[math.MaxInt64]`,
logical length 1. That is the same shape as a one-element slice of a
`MaxInt64`-length run, and it is valid on 32-bit. The test now also checks the
concatenated run ends (`[5, 6]`) and values (`[1, 2]`), not just the length.
- The two `*OverflowRunEndEncoding` tests skip their `int64` case when
`tt.max > math.MaxInt`, i.e. only where the inputs cannot be constructed. The
int16 and int32 cases still run everywhere.
### Are these changes tested?
- `GOARCH=386 go test ./arrow/array -run Concat`: ok (two `int64` subtests
skipped).
- `go test ./arrow/array`: ok on amd64, no skips.
- The rewritten clamp test still guards #1219: with #1219's `concat.go`
change reverted it fails on amd64 (`overflow in run-length-encoded run ends
concat`) and on 386 (wrong run ends).
- `go vet ./arrow/array` and `gofmt -l`: clean.
Not addressed here: `arrow/array` still has other, unrelated 386 failures in
the decimal tests (e.g. `TestDecimal128Slice`, `TestDecimal256Slice`), which
fail the same way on main.
### Are there any user-facing changes?
No.
--
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]
