zeroshade commented on code in PR #956:
URL: https://github.com/apache/arrow-go/pull/956#discussion_r3604452003
##########
arrow/compute/internal/kernels/scalar_comparisons.go:
##########
@@ -55,24 +55,28 @@ func comparePrimitiveArrayArray[T arrow.FixedWidthType](op
cmpFn[T, T]) binaryKe
left = arrow.GetData[T](leftBytes)
right = arrow.GetData[T](rightBytes)
nvals = len(left)
- nbatches = nvals / batchSize
tmpOutput [batchSize]uint32
)
tmpOutSlice := tmpOutput[:]
if prefix := offset % 8; prefix != 0 {
Review Comment:
This prefix handling is correct for the pure-Go path — but the same fix
needs to land in `_lib/scalar_comparison.cc` and the regenerated `*_amd64.s`,
which is what the default amd64 build actually dispatches to. On amd64,
`num_batches`/tail are still derived from the original `length`, so a non-zero
output offset still over-reads / corrupts bits. See the main review comment for
the full ask.
--
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]
