wgtmac commented on code in PR #50807:
URL: https://github.com/apache/arrow/pull/50807#discussion_r3906082519
##########
cpp/src/parquet/statistics_test.cc:
##########
@@ -1509,13 +1539,19 @@ void TestFloatStatistics<Float16LogicalType>::Init() {
template <typename T>
NodePtr TestFloatStatistics<T>::MakeNode(const std::string& name,
Repetition::type rep) {
- return PrimitiveNode::Make(name, rep, ParquetType::type_num);
+ auto node = PrimitiveNode::Make(name, rep, ParquetType::type_num);
+ std::static_pointer_cast<PrimitiveNode>(node)->SetColumnOrder(
+ ColumnOrder::type_defined_);
+ return node;
}
template <>
NodePtr TestFloatStatistics<Float16LogicalType>::MakeNode(const std::string&
name,
Repetition::type
rep) {
- return PrimitiveNode::Make(name, rep, LogicalType::Float16(),
- Type::FIXED_LEN_BYTE_ARRAY, 2);
+ auto node = PrimitiveNode::Make(name, rep, LogicalType::Float16(),
Review Comment:
nit: should we expand `PrimitiveNode::Make` to accept column order as well?
Currently we cannot create a primitive node of floating type with
type-defined-order in a single shot.
##########
cpp/src/parquet/statistics.h:
##########
@@ -259,6 +274,29 @@ class PARQUET_EXPORT Statistics {
std::optional<bool> is_max_value_exact,
::arrow::MemoryPool* pool = ::arrow::default_memory_pool());
+ /// \brief Create a new statistics instance given a column schema
+ /// definition and preexisting state
+ /// \param[in] descr the column schema
+ /// \param[in] encoded_min the encoded minimum value
+ /// \param[in] encoded_max the encoded maximum value
+ /// \param[in] num_values total number of values
+ /// \param[in] null_count number of null values
+ /// \param[in] distinct_count number of distinct values
+ /// \param[in] nan_count number of NaN values, if available
+ /// \param[in] has_min_max whether the min/max statistics are set
+ /// \param[in] has_null_count whether the null_count statistics are set
+ /// \param[in] has_distinct_count whether the distinct_count statistics are
set
+ /// \param[in] is_min_value_exact whether the min value is exact
+ /// \param[in] is_max_value_exact whether the max value is exact
+ /// \param[in] pool a memory pool to use for any memory allocations, optional
+ static std::shared_ptr<Statistics> Make(
Review Comment:
I think that's a different concern. Here I mean we don't have to add yet
another overload for adding an optional nan_count because we can add it to the
end of an existing one with a default value. We don't need a breaking change
this time. The issue you've mentioned is to replace `has_xxx` and `xxx` with a
single `std::optional<int64_t>` which has to be a breaking change.
--
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]
