wgtmac commented on code in PR #50694:
URL: https://github.com/apache/arrow/pull/50694#discussion_r3670451713
##########
cpp/src/parquet/metadata.cc:
##########
@@ -94,9 +94,11 @@ namespace {
template <typename DType>
std::shared_ptr<Statistics> MakeTypedColumnStats(const format::ColumnMetaData&
metadata,
+ const EncodedStatistics&
encoded_stats,
const ColumnDescriptor* descr,
::arrow::MemoryPool* pool) {
const auto& statistics = metadata.statistics;
+ const int64_t null_count = encoded_stats.has_null_count ?
encoded_stats.null_count : 0;
Review Comment:
Why can't we directly use `metadata.statistics`?
##########
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:
This silently changes the public behavior that corrupted values are no
longer visible to users from reading a file. I think we have a few options
w.r.t. values in the stats and page indexes:
1. Reject any corrupted values by throwing an exception (@etseidl has
proposed this in another thread)
2. Regard corrupted values as unset (this PR does).
3. Preserve corrupted values but provide a `Validate` function or a function
to fix the corrupted values.
4. Do nothing so it is users' responsibility to check corrupted values and
take action (ignore them or error out).
My understanding is that users have to validate these values before
consumption anyway. In that case, an unset or corrupted value does not make too
much difference. Users may still want to read the file even when there are
corrupted stats (or even columns/row groups), e.g. recover corrupted/legacy
files. So I think we shouldn't error out on invalid stats. Preserving corrupted
values still bring some values like help inspecting the files so here might be
the best place to fix this issue?
WDYT? @pitrou @mapleFU @emkornfield
--
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]
