zeroshade commented on code in PR #1184:
URL: https://github.com/apache/arrow-go/pull/1184#discussion_r3786583560
##########
arrow/array/compare.go:
##########
@@ -361,6 +361,35 @@ func SliceEqual(left arrow.Array, lbeg, lend int64, right
arrow.Array, rbeg, ren
return Equal(l, r)
}
+type listOffset interface {
+ int32 | int64
+}
+
+func arrayEqualListOffsets[T listOffset](leftValues, rightValues arrow.Array,
+ leftOffsets, rightOffsets []T, leftOffset, rightOffset, length int,
validBits []byte) bool {
+ return bitutils.VisitSetBitRuns(validBits, int64(leftOffset),
int64(length),
Review Comment:
`IsNull` and `IsValid` treat both `nil` and a zero-length validity buffer as
all-valid, but `VisitSetBitRuns` special-cases only `nil`. Consequently, a
nonempty list created through `NewData` with a non-nil empty validity buffer
now panics here when it is the left operand, while reversing the operands
returns `true`. The previous implementation returned `true` in both directions.
Although `Validate` rejects this array shape, this is still a regression in
`Equal` behavior for data constructible through the exported API. Please
normalize `len(validBits) == 0` to the all-valid path, apply the same handling
in `arrayEqualFixedSizeList`, and add regression coverage for both operand
orders.
--
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]
