bkietz commented on a change in pull request #9294:
URL: https://github.com/apache/arrow/pull/9294#discussion_r566359504
##########
File path: cpp/src/arrow/compute/cast.cc
##########
@@ -225,13 +208,21 @@ Result<std::shared_ptr<CastFunction>> GetCastFunction(
}
bool CanCast(const DataType& from_type, const DataType& to_type) {
- // TODO
internal::EnsureInitCastTable();
- auto it = internal::g_cast_table.find(static_cast<int>(from_type.id()));
+ auto it = internal::g_cast_table.find(static_cast<int>(to_type.id()));
if (it == internal::g_cast_table.end()) {
return false;
}
- return it->second->CanCastTo(to_type);
+
+ const CastFunction* function = it->second.get();
+ DCHECK_EQ(function->out_type_id(), to_type.id());
+
+ for (auto from_id : function->in_type_ids()) {
+ // XXX should probably check the output type as well
Review comment:
For example `CanCast(*list(null()), *list(list(int32())))` currently
returns true, even though that cast is not supported. We support *some* casts
between list types but since we only check type_ids there's no way to express
more than "Type::LIST -> Type::LIST is supported"
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
