etseidl commented on code in PR #50694:
URL: https://github.com/apache/arrow/pull/50694#discussion_r3675024642
##########
cpp/src/parquet/thrift_internal.h:
##########
@@ -306,10 +306,10 @@ static inline EncodedStatistics FromThrift(const
format::Statistics& stats,
out.set_min(stats.min);
}
}
- if (stats.__isset.null_count) {
+ if (stats.__isset.null_count && stats.null_count >= 0) {
Review Comment:
I see this was closed, but would just like to say that the situation here is
a little different from arrow-rs. It seems arrow-cpp keeps the sign for
`null_count` and `nan_count`, so returning the actual encoded value is probably
the best thing to do. Let users decide what to make of that. arrow-rs, for
unknown historical reasons, changes these fields to unsigned, so when a
conversion to unsigned is not possible, the IMO most reasonable thing to do is
return an error to let the user know something is wrong with the metadata. I
don't think silently ignoring the error in that case is an option.
--
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]
