lidavidm commented on a change in pull request #11368:
URL: https://github.com/apache/arrow/pull/11368#discussion_r745915098
##########
File path: cpp/src/arrow/compute/kernels/scalar_set_lookup.cc
##########
@@ -184,7 +184,27 @@ struct InitStateVisitor {
}
Result<std::unique_ptr<KernelState>> GetResult() {
- if (!options.value_set.type()->Equals(arg_type)) {
+ if (arg_type->id() == Type::TIMESTAMP &&
+ options.value_set.type()->id() == Type::TIMESTAMP) {
+ // Other types will fail when casting, so no separate check is needed
+ const auto& ty1 = checked_cast<const TimestampType&>(*arg_type);
+ const auto& ty2 = checked_cast<const
TimestampType&>(*options.value_set.type());
+ if (ty1.timezone().empty() ^ ty2.timezone().empty()) {
+ return Status::Invalid(
+ "Cannot compare timestamp with timezone to timestamp without
timezone, got: ",
+ ty1, " and ", ty2);
+ }
+ } else if (arg_type->id() == Type::STRING &&
+ !is_base_binary_like(options.value_set.type()->id())) {
+ // This is a bit of a hack, but don't implicitly cast from a non-binary
+ // type to string, since most types support casting to string and that
+ // may lead to surprises. However, we do want most other implicit casts.
Review comment:
It also applies to LARGE_STRING, updated the conditional.
--
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]
