wgtmac commented on code in PR #50807:
URL: https://github.com/apache/arrow/pull/50807#discussion_r3718686847
##########
cpp/src/parquet/statistics.h:
##########
@@ -134,11 +134,13 @@ class PARQUET_EXPORT EncodedStatistics {
int64_t null_count = 0;
int64_t distinct_count = 0;
+ int64_t nan_count = 0;
Review Comment:
I would suggest directly using `std::optional<int64_t> nan_count` just like
`is_max_value_exact` and `is_min_value_exact`.
##########
cpp/src/parquet/types.h:
##########
@@ -125,7 +125,7 @@ struct Repetition {
// Parquet file. These stats are discarded for types that need unsigned.
// See PARQUET-686.
struct SortOrder {
- enum type { SIGNED, UNSIGNED, UNKNOWN };
+ enum type { SIGNED, UNSIGNED, TOTAL_ORDER, UNKNOWN };
Review Comment:
Why do we need this change? Shouldn't the new `IEEE_754_TOTAL_ORDER` reuse
``SortOrder::SIGNED` ?
##########
cpp/src/parquet/schema.h:
##########
@@ -376,17 +376,15 @@ class PARQUET_EXPORT ColumnDescriptor {
ColumnOrder column_order() const { return primitive_node_->column_order(); }
- SortOrder::type sort_order() const {
- const auto& la = logical_type();
- auto pt = physical_type();
- return la ? GetSortOrder(la, pt) : GetSortOrder(converted_type(), pt);
- }
+ SortOrder::type sort_order() const;
// Whether ColumnOrder-governed min/max values have a supported ordering.
bool can_use_min_max() const {
switch (column_order().get_order()) {
case ColumnOrder::TYPE_DEFINED_ORDER:
return sort_order() != SortOrder::UNKNOWN;
+ case ColumnOrder::IEEE_754_TOTAL_ORDER:
+ return sort_order() == SortOrder::TOTAL_ORDER;
Review Comment:
IIUC, sort order only applies to TYPE_DEFINED_ORDER. We don't even need to
check it here.
##########
cpp/src/parquet/schema.cc:
##########
@@ -620,6 +620,12 @@ void ToParquet(const GroupNode* schema,
std::vector<format::SchemaElement>* out)
schema->VisitConst(&visitor);
}
+bool IsFloatingPoint(const ColumnDescriptor& descr) {
+ return descr.physical_type() == Type::FLOAT || descr.physical_type() ==
Type::DOUBLE ||
+ (descr.physical_type() == Type::FIXED_LEN_BYTE_ARRAY &&
descr.logical_type() &&
Review Comment:
Do we need to check type length for float16?
--
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]
