Copilot commented on code in PR #50892:
URL: https://github.com/apache/arrow/pull/50892#discussion_r3910337506
##########
cpp/src/parquet/arrow/schema.cc:
##########
@@ -1059,9 +1094,16 @@ Result<bool> ApplyOriginalStorageMetadata(const Field&
origin_field,
// Apply original metadata recursively to children
for (int i = 0; i < inferred_type->num_fields(); ++i) {
+ std::shared_ptr<::arrow::Field> origin_child;
+ if (match_children_by_name) {
+ origin_child = checked_cast<const ::arrow::StructType&>(*origin_type)
+ .GetFieldByName(inferred_type->field(i)->name());
+ } else {
+ origin_child = origin_type->field(i);
+ }
ARROW_ASSIGN_OR_RAISE(
const bool child_modified,
- ApplyOriginalMetadata(*origin_type->field(i),
&inferred->children[i]));
+ ApplyOriginalMetadata(*origin_child, &inferred->children[i]));
Review Comment:
When matching children by name, `origin_child` can be null (e.g. if the
origin struct doesn't contain a field with the inferred name). It is then
unconditionally dereferenced in `ApplyOriginalMetadata(*origin_child, ...)`,
which can crash in release builds (since `checked_cast` becomes a
`static_cast`). Add an explicit null check (and ideally guard that name-based
matching is only used for struct types) and return a helpful error instead of
dereferencing a null pointer.
This issue also appears on line 1195 of the same file.
--
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]
