zeroshade commented on code in PR #1174:
URL: https://github.com/apache/arrow-go/pull/1174#discussion_r3896343961
##########
arrow/array/binary.go:
##########
@@ -432,17 +439,120 @@ func (a *LargeBinary) ValidateFull() error {
}
func arrayEqualLargeBinary(left, right *LargeBinary) bool {
- for i := 0; i < left.Len(); i++ {
- if left.IsNull(i) {
- continue
+ if useScalarVariableWidthEquality(left) {
+ for i := range left.Len() {
+ if !left.IsNull(i) && !bytes.Equal(left.Value(i),
right.Value(i)) {
+ return false
+ }
+ }
+ return true
+ }
+ return arrayEqualVariableWidth(
+ left.valueOffsets, right.valueOffsets,
+ left.valueBytes, right.valueBytes,
+ left.Offset(), right.Offset(), left.Len(),
+ left.NullN(), left.NullBitmapBytes(),
+ bytes.Equal,
+ )
+}
+
+type binaryOffset interface {
+ ~int32 | ~int64
+}
+
+func useScalarVariableWidthEquality(values arrow.Array) bool {
+ if values.NullN() == 0 {
+ return false
+ }
+ if values.Len() <= 64 || len(values.NullBitmapBytes()) == 0 {
+ return true
+ }
+
+ // Very short validity runs cost more to set up than direct value
comparisons.
+ // Sample a few runs and retain the scalar path when they average under
four values.
+ const (
+ sampleRuns = 8
+ minAverageRunLength = 4
+ )
+ runs := bitutils.NewSetBitRunReader(
+ values.NullBitmapBytes(), int64(values.Data().Offset()),
int64(values.Len()),
+ )
+ validValues := int64(0)
+ for range sampleRuns {
+ run := runs.NextRun()
+ if run.Length == 0 {
+ return false
+ }
+ validValues += run.Length
+ }
+ return validValues < sampleRuns*minAverageRunLength
+}
+
+func arrayEqualVariableWidth[T binaryOffset, V ~[]byte | ~string](
+ leftOffsets, rightOffsets []T,
+ leftValues, rightValues V,
+ leftOffset, rightOffset, length, nulls int,
+ validity []byte,
+ equalValues func(V, V) bool,
+) bool {
+ if nulls == 0 {
Review Comment:
The `nulls == 0` fast path compares every payload byte even when a validity
bitmap contains nulls. With inconsistent `ArrayData`—a bitmap containing nulls
but declared `null_count=0`—payload differences in null slots now make arrays
unequal, unlike the prior scalar implementation. Please only use the
whole-payload path when no validity bitmap exists, or otherwise process bitmap
runs, and add a regression test.
--
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]
