pitrou commented on code in PR #50513:
URL: https://github.com/apache/arrow/pull/50513#discussion_r3628522049
##########
cpp/src/arrow/compute/kernels/vector_hash.cc:
##########
@@ -700,6 +701,13 @@ void AddHashKernels(VectorFunction* func, VectorKernel
base, OutputType out_ty)
DCHECK_OK(func->AddKernel(base));
}
+ // float16() is not part of PrimitiveTypes() (FloatingPointTypes() only
covers
Review Comment:
Can we open a separate issue for `FloatingPointTypes`? I think we should
make `float16` part of it.
##########
cpp/src/arrow/compute/kernels/vector_hash_test.cc:
##########
@@ -403,6 +403,44 @@ TEST_F(TestHashKernel, DictEncodeBoolean) {
ArrayFromJSON(boolean(), "[true]"), ArrayFromJSON(int32(), "[0, null,
0]"));
}
+// float16 is not part of PrimitiveTypes(), so it is not covered by
+// TestHashKernelPrimitive above and gets its own coverage here. Like float32
and
+// float64, it is hashed by its raw bit pattern.
Review Comment:
Well, can we fix `TestHashKernelPrimitive` so that it also covers
`HalfFloatType`?
--
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]
