pitrou commented on code in PR #47466:
URL: https://github.com/apache/arrow/pull/47466#discussion_r2322567210
##########
cpp/src/parquet/statistics.cc:
##########
@@ -991,6 +996,8 @@ std::shared_ptr<Comparator> DoMakeComparator(Type::type
physical_type,
default:
ParquetException::NYI("Unsigned Compare not implemented");
}
+ } else if (SortOrder::UNKNOWN == sort_order) {
+ return nullptr;
Review Comment:
Ok, I looked at this and am coming to the following suggestion:
1. `DoMakeComparator` should keep throwing an exception instead of returning
`nullptr`
2. `TypedStatisticsImpl` should catch the error when calling
`MakeComparator` and then simply set the comparator to `nullptr` (methods using
the comparator should be changed to be no-ops when that happens)
3. Also, `TypedStatisticsImpl` only uses the comparator when
updating/writing statistics, so it could be probably be created lazily, not in
the constructor? (but this is more of an improvement and not strictly necessary)
--
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]
