pitrou commented on code in PR #49718:
URL: https://github.com/apache/arrow/pull/49718#discussion_r3074030367
##########
cpp/src/arrow/extension/fixed_shape_tensor.cc:
##########
@@ -133,15 +144,36 @@ Result<std::shared_ptr<DataType>>
FixedShapeTensorType::Deserialize(
}
std::vector<std::string> dim_names;
if (document.HasMember("dim_names")) {
- for (auto& x : document["dim_names"].GetArray()) {
+ const auto& json_dim_names = document["dim_names"];
+ if (!json_dim_names.IsArray()) {
+ return Status::Invalid("dim_names must be an array");
+ }
+ for (const auto& x : json_dim_names.GetArray()) {
+ if (!x.IsString()) {
+ return Status::Invalid("dim_names must contain strings");
+ }
dim_names.emplace_back(x.GetString());
}
if (shape.size() != dim_names.size()) {
return Status::Invalid("Invalid dim_names");
}
}
- return fixed_shape_tensor(value_type, shape, permutation, dim_names);
+ // Validate product of shape dimensions matches storage type list_size.
+ // This check is intentionally after field parsing so that metadata-level
errors
+ // (type mismatches, size mismatches) are reported first.
+ ARROW_ASSIGN_OR_RAISE(auto ext_type, FixedShapeTensorType::Make(
+ value_type, shape, permutation,
dim_names));
+ const auto& fst_type = internal::checked_cast<const
FixedShapeTensorType&>(*ext_type);
+ const int64_t expected_size =
+ std::accumulate(fst_type.shape().begin(), fst_type.shape().end(),
+ static_cast<int64_t>(1), std::multiplies<>());
Review Comment:
I suppose multiplication could overflow here, can we check that (we have
`MultiplyWithOverflow`)?
--
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]
