zeroshade commented on code in PR #1488:
URL: https://github.com/apache/arrow-go/pull/1488#discussion_r4515202676
##########
arrow/compute/internal/kernels/scalar_comparisons.go:
##########
@@ -848,12 +862,62 @@ func isValidAt(span *exec.ArraySpan, i int64) bool {
return true
}
+// setNaNBits sets bit i of out to v for each NaN slot i of a float16, float32
+// or float64 span, and does nothing for other types.
+func setNaNBits(input *exec.ArraySpan, out []byte, v bool) {
Review Comment:
nit (non-blocking): this sets one bit per call. A word-at-a-time version
would be faster on large inputs, but I don't think it's worth the extra
complexity here.
##########
arrow/compute/internal/kernels/scalar_comparisons.go:
##########
@@ -742,21 +743,34 @@ func dictValidity(ctx *exec.KernelCtx, input
*exec.ArraySpan, out *exec.ExecResu
default:
return fmt.Errorf("%w: unsupported dictionary index type",
arrow.ErrInvalid)
}
+
+ dict := input.Dictionary()
+ valueValidity, valueOffset := dict.Buffers[0].Buf, dict.Offset
+ if nanIsNull && arrow.IsFloating(dict.Type.ID()) {
+ buf := ctx.AllocateBitmap(dict.Len)
Review Comment:
nit (non-blocking): this rebuilds a bitmap the size of the dictionary on
every batch. That's fine for typical dictionaries; caching it would only help
with very large dictionaries spread over many batches, so I'm happy to leave it.
--
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]
