fix(arrow/array): widen dictionary index bounds checks

[fallintoplace]

Rationale for this change

Signed dictionary index validation converts the dictionary length to the index type before comparing it with the largest index. A dictionary with 128 entries therefore turns the upper bound into -128 for int8 and rejects valid indices.

What changes are included in this PR?

Compare signed index values after widening them to uint64. Add coverage for a valid int8 index at the dictionary size boundary.

Are these changes tested?

• go test ./arrow/array

Are there any user-facing changes?

Valid signed dictionary indices at the index type boundary are now accepted. Negative and out-of-bounds indices continue to return errors.

[zeroshade]

Correct fix — and the bug is somewhat bigger than the description suggests.

max >= int8(upperlimit) only holds while upperlimit ≤ 127. Past that the conversion wraps: int8(128) is -128, so max >= -128 is true for every possible index and validation rejects everything. So it isn't only a dictionary of exactly 128 entries — any int8-indexed dictionary with 128 or more entries was unusable, and likewise int16 past 32768 and so on. Since an int8 index can only address 0–127 regardless, those dictionaries were being rejected purely because of the cast.

The uint64(max) >= upperlimit form is right. The subtle part is why the cast is safe: || short-circuits, so the second operand is only reached when min >= 0, and min <= max then guarantees max >= 0. The widening is always faithful and no negative value can become a huge uint64.

I checked the one spot that could have regressed — if GetMinMaxInt8 returned sentinels for an empty slice, uint64(max) on a negative sentinel would be enormous and would start failing empty index arrays that previously passed. That can't happen: checkIndexBounds returns early on indices.length == 0, so the slice is always non-empty.

Error paths stay covered too — TestDictionaryFromArrays already exercises negative and out-of-bounds indices, so the claim that those still error is backed by existing tests.

One more coverage note beyond the inline comment: only INT8 gets a regression test, but the identical bug was fixed in INT16, INT32 and INT64. A table-driven case across the four signed types would be cheap and would pin all four against a future refactor.

---

Note: this review was drafted with AI assistance by a maintainer and may contain mistakes. If anything here looks wrong, say so on the PR and I'll take another look.

[zeroshade]

Minor: this asserts the error is nil but never checks the result, so a (nil, nil) return would pass the test just as happily. require.NotNil(t, result) would close that gap — and then the if result != nil guard below becomes unnecessary.