zeroshade commented on code in PR #1171:
URL: https://github.com/apache/arrow-go/pull/1171#discussion_r3808009115
##########
arrow/array/numeric_generic.go:
##########
@@ -449,7 +451,49 @@ func NewDate64Data(data arrow.ArrayData) *Date64 {
func (a *Date64) Date64Values() []arrow.Date64 { return a.Values() }
-func arrayEqualFixedWidth[T arrow.FixedWidthType](left, right
arrow.TypedArray[T]) bool {
+type fixedWidthArray[T arrow.FixedWidthType] interface {
+ arrow.TypedArray[T]
+ Values() []T
+}
+
+func arrayEqualFixedWidth[T arrow.FixedWidthType](left, right
fixedWidthArray[T]) bool {
+ // Avoid the fixed cost of bytes.Equal for very small arrays.
+ if left.Len() < 8 {
+ return arrayEqualFixedWidthScalar(left, right)
+ }
+
+ leftValues := left.Values()
+ rightValues := right.Values()
+ if left.NullN() == 0 {
Review Comment:
**Nonblocking:** the follow-up now compares attached validity bitmaps even
when `NullN() == 0`, but this fast path still treats `NullN()` as authoritative.
With two arrays that both declare `null_count = 0`, carry the same bitmap
with an unset bit, and differ only in that null slot:
- merge base: equal
- current bulk path: unequal
- current scalar path for `Len() < 8`: equal
IPC and C Data imports can expose such inconsistent metadata because the
declared count is not cross-checked against the bitmap.
This is nonblocking because the input violates Arrow's format invariants,
but it leaves the new “respect attached validity bitmaps” behavior incomplete
and length-dependent. If the bitmap should remain authoritative when present,
gate the raw `bytes.Equal` path on an absent bitmap and add the complementary
regression test. Otherwise, please document that `NullN() == 0` intentionally
overrides an attached bitmap.
--
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]
