Skip to content

Commit bb83ede

Browse files
authored
feat: Collect Parquet NaN metrics during writes (#727)
Collects NaN value counts for float and double columns during Parquet writes, since the Parquet footer statistics do not track NaN counts. ## Changes - **Write-side NaN metric collection** (`FieldMetricsCollector`): A visitor that walks each record batch before writing, accumulating value counts, null counts, NaN counts, and NaN-excluding lower/upper bounds for float/double fields. - **MetricsConfig-aware skipping**: Fields whose `MetricsMode` is `kNone` are skipped entirely, avoiding wasted work. - **Integration with existing footer metrics**: Write-side `FieldMetrics` take precedence over footer statistics in `ParquetMetrics::GetMetrics`, so NaN counts are populated while counts/bounds still fall back to footer stats when write-side data isn't available. - **Tests**: `ParquetMetricsTest` now overrides `ReportsNanCounts()` to `true`, and existing NaN test cases verify NaN counts alongside existing value/null count assertions. ## Behavior alignment with Java - Fields nested inside lists/maps do not get NaN metrics (both Java and C++ agree — Java collects then discards; C++ skips collection entirely). - NaN values are excluded from lower/upper bounds in both implementations. - Float/double fields with all-NaN values correctly set `nan_value_count` without setting bounds.
1 parent cb79f29 commit bb83ede

4 files changed

Lines changed: 171 additions & 10 deletions

File tree

src/iceberg/parquet/parquet_writer.cc

Lines changed: 155 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,16 @@
1919

2020
#include "iceberg/parquet/parquet_writer.h"
2121

22+
#include <cmath>
23+
#include <cstdint>
2224
#include <memory>
25+
#include <optional>
2326
#include <string_view>
27+
#include <type_traits>
28+
#include <unordered_map>
2429
#include <vector>
2530

31+
#include <arrow/array.h>
2632
#include <arrow/c/bridge.h>
2733
#include <arrow/record_batch.h>
2834
#include <arrow/util/compression.h>
@@ -36,7 +42,9 @@
3642
#include "iceberg/arrow/arrow_status_internal.h"
3743
#include "iceberg/parquet/parquet_metrics_internal.h"
3844
#include "iceberg/schema_internal.h"
45+
#include "iceberg/type.h"
3946
#include "iceberg/util/macros.h"
47+
#include "iceberg/util/visit_type.h"
4048

4149
namespace iceberg::parquet {
4250

@@ -74,6 +82,144 @@ Status CheckCompressionAvailable(std::string_view compression_name,
7482
return {};
7583
}
7684

85+
template <typename ArrowArrayType, typename ValueType>
86+
Status UpdateFloatingFieldMetrics(int32_t field_id, const ::arrow::Array& arrow_array,
87+
const std::vector<uint8_t>* valid_rows,
88+
std::unordered_map<int32_t, FieldMetrics>& metrics) {
89+
constexpr auto expected_type_id =
90+
std::is_same_v<ValueType, float> ? ::arrow::Type::FLOAT : ::arrow::Type::DOUBLE;
91+
ICEBERG_PRECHECK(arrow_array.type_id() == expected_type_id,
92+
"Expected Arrow floating-point array for field metrics collection");
93+
const auto& array = static_cast<const ArrowArrayType&>(arrow_array);
94+
auto& field_metrics = metrics[field_id];
95+
field_metrics.field_id = field_id;
96+
if (field_metrics.value_count < 0) {
97+
field_metrics.value_count = 0;
98+
}
99+
if (field_metrics.null_value_count < 0) {
100+
field_metrics.null_value_count = 0;
101+
}
102+
if (field_metrics.nan_value_count < 0) {
103+
field_metrics.nan_value_count = 0;
104+
}
105+
106+
field_metrics.value_count += array.length();
107+
108+
for (int64_t i = 0; i < array.length(); ++i) {
109+
if ((valid_rows != nullptr && (*valid_rows)[i] == 0) || array.IsNull(i)) {
110+
++field_metrics.null_value_count;
111+
continue;
112+
}
113+
114+
ValueType value = array.Value(i);
115+
if (std::isnan(value)) {
116+
++field_metrics.nan_value_count;
117+
continue;
118+
}
119+
120+
auto literal = [&]() {
121+
if constexpr (std::is_same_v<ValueType, float>) {
122+
return Literal::Float(value);
123+
} else {
124+
return Literal::Double(value);
125+
}
126+
}();
127+
if (!field_metrics.lower_bound.has_value() ||
128+
literal < field_metrics.lower_bound.value()) {
129+
field_metrics.lower_bound = literal;
130+
}
131+
if (!field_metrics.upper_bound.has_value() ||
132+
literal > field_metrics.upper_bound.value()) {
133+
field_metrics.upper_bound = std::move(literal);
134+
}
135+
}
136+
137+
return {};
138+
}
139+
140+
std::optional<std::vector<uint8_t>> BuildValidRows(const ::arrow::Array& array,
141+
const std::vector<uint8_t>* parent) {
142+
if (parent == nullptr && array.null_count() == 0) {
143+
return std::nullopt;
144+
}
145+
146+
std::vector<uint8_t> valid_rows(array.length(), 1);
147+
for (int64_t i = 0; i < array.length(); ++i) {
148+
if ((parent != nullptr && (*parent)[i] == 0) || array.IsNull(i)) {
149+
valid_rows[i] = 0;
150+
}
151+
}
152+
return valid_rows;
153+
}
154+
155+
class FieldMetricsCollector {
156+
public:
157+
FieldMetricsCollector(std::unordered_map<int32_t, FieldMetrics>& metrics,
158+
const MetricsConfig& metrics_config, const Schema& schema)
159+
: metrics_(metrics), metrics_config_(metrics_config), schema_(schema) {}
160+
161+
Status VisitStruct(const StructType& type, const ::arrow::Array& array) {
162+
ICEBERG_PRECHECK(array.type_id() == ::arrow::Type::STRUCT,
163+
"Expected Arrow struct array for Iceberg struct metrics collection");
164+
const auto& struct_array = static_cast<const ::arrow::StructArray&>(array);
165+
ICEBERG_PRECHECK(
166+
struct_array.num_fields() == type.fields().size(),
167+
"Arrow struct field count does not match Iceberg struct field count");
168+
169+
for (int i = 0; i < struct_array.num_fields(); ++i) {
170+
ICEBERG_RETURN_UNEXPECTED(VisitField(type.fields()[i], *struct_array.field(i)));
171+
}
172+
return {};
173+
}
174+
175+
Status VisitList(const ListType& /*type*/, const ::arrow::Array& /*array*/) {
176+
return {};
177+
}
178+
179+
Status VisitMap(const MapType& /*type*/, const ::arrow::Array& /*array*/) { return {}; }
180+
181+
Status VisitPrimitive(const PrimitiveType& type, const ::arrow::Array& array) {
182+
switch (type.type_id()) {
183+
case TypeId::kFloat:
184+
return UpdateFloatingFieldMetrics<::arrow::FloatArray, float>(
185+
field_id_, array, valid_rows_, metrics_);
186+
case TypeId::kDouble:
187+
return UpdateFloatingFieldMetrics<::arrow::DoubleArray, double>(
188+
field_id_, array, valid_rows_, metrics_);
189+
default:
190+
return {};
191+
}
192+
}
193+
194+
private:
195+
Status VisitField(const SchemaField& field, const ::arrow::Array& array) {
196+
// Skip metrics collection for fields whose mode is kNone in MetricsConfig.
197+
ICEBERG_ASSIGN_OR_RAISE(auto column_name,
198+
schema_.FindColumnNameById(field.field_id()));
199+
if (column_name.has_value() && metrics_config_.ColumnMode(column_name.value()).kind ==
200+
MetricsMode::Kind::kNone) {
201+
return {};
202+
}
203+
204+
auto previous_valid_rows = valid_rows_;
205+
auto field_valid_rows = BuildValidRows(array, previous_valid_rows);
206+
if (field_valid_rows.has_value()) {
207+
valid_rows_ = &field_valid_rows.value();
208+
}
209+
210+
field_id_ = field.field_id();
211+
auto status = VisitTypeCategory(*field.type(), this, array);
212+
valid_rows_ = previous_valid_rows;
213+
return status;
214+
}
215+
216+
std::unordered_map<int32_t, FieldMetrics>& metrics_;
217+
const MetricsConfig& metrics_config_;
218+
const Schema& schema_;
219+
const std::vector<uint8_t>* valid_rows_ = nullptr;
220+
int32_t field_id_ = -1;
221+
};
222+
77223
Result<std::optional<int32_t>> ParseCodecLevel(const WriterProperties& properties) {
78224
auto level_str = properties.Get(WriterProperties::kParquetCompressionLevel);
79225
if (level_str.empty()) {
@@ -136,6 +282,12 @@ class ParquetWriter::Impl {
136282
ICEBERG_ARROW_ASSIGN_OR_RETURN(auto batch,
137283
::arrow::ImportRecordBatch(array, arrow_schema_));
138284

285+
ICEBERG_ARROW_ASSIGN_OR_RETURN(auto struct_array, batch->ToStructArray());
286+
FieldMetricsCollector field_metrics_collector(field_metrics_, *metrics_config_,
287+
*schema_);
288+
ICEBERG_RETURN_UNEXPECTED(
289+
field_metrics_collector.VisitStruct(*schema_, *struct_array));
290+
139291
ICEBERG_ARROW_RETURN_NOT_OK(writer_->WriteRecordBatch(*batch));
140292

141293
return {};
@@ -179,9 +331,8 @@ class ParquetWriter::Impl {
179331
ICEBERG_PRECHECK(writer_ == nullptr, "Cannot return metrics for unclosed writer");
180332
ICEBERG_PRECHECK(metadata_ != nullptr,
181333
"Cannot return metrics because Parquet metadata is not available");
182-
// TODO(WZhuo): collect write-side FieldMetrics to support NaN value counts.
183334
return ParquetMetrics::GetMetrics(*schema_, *parquet_schema_, *metrics_config_,
184-
*metadata_);
335+
*metadata_, field_metrics_);
185336
}
186337

187338
private:
@@ -205,6 +356,8 @@ class ParquetWriter::Impl {
205356
int64_t total_bytes_{0};
206357
// Row group start offsets in the Parquet file.
207358
std::vector<int64_t> split_offsets_;
359+
// Write-side metrics for fields whose Parquet footer metrics are incomplete.
360+
std::unordered_map<int32_t, FieldMetrics> field_metrics_;
208361
};
209362

210363
ParquetWriter::~ParquetWriter() = default;

src/iceberg/test/metrics_test_base.cc

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -541,8 +541,9 @@ void MetricsTestBase::MetricsForNaNColumns() {
541541

542542
ASSERT_TRUE(metrics.row_count.has_value()) << "row_count should be set";
543543
EXPECT_EQ(*metrics.row_count, 2);
544-
AssertCounts(1, 2, 0, metrics);
545-
AssertCounts(2, 2, 0, metrics);
544+
auto expected_nan_count = ReportsNanCounts() ? std::optional<int64_t>(2) : std::nullopt;
545+
AssertCounts(1, 2, 0, expected_nan_count, metrics);
546+
AssertCounts(2, 2, 0, expected_nan_count, metrics);
546547

547548
// When all values are NaN, bounds should not be set
548549
AssertBounds<float>(1, float32(), std::nullopt, std::nullopt, metrics);
@@ -579,8 +580,9 @@ void MetricsTestBase::ColumnBoundsWithNaNValueAtFront() {
579580

580581
ASSERT_TRUE(metrics.row_count.has_value()) << "row_count should be set";
581582
EXPECT_EQ(*metrics.row_count, 3);
582-
AssertCounts(1, 3, 0, metrics);
583-
AssertCounts(2, 3, 0, metrics);
583+
auto expected_nan_count = ReportsNanCounts() ? std::optional<int64_t>(1) : std::nullopt;
584+
AssertCounts(1, 3, 0, expected_nan_count, metrics);
585+
AssertCounts(2, 3, 0, expected_nan_count, metrics);
584586

585587
// Bounds should be computed from non-NaN values
586588
if (metrics.lower_bounds.contains(1)) {
@@ -619,8 +621,9 @@ void MetricsTestBase::ColumnBoundsWithNaNValueInMiddle() {
619621

620622
ASSERT_TRUE(metrics.row_count.has_value()) << "row_count should be set";
621623
EXPECT_EQ(*metrics.row_count, 3);
622-
AssertCounts(1, 3, 0, metrics);
623-
AssertCounts(2, 3, 0, metrics);
624+
auto expected_nan_count = ReportsNanCounts() ? std::optional<int64_t>(1) : std::nullopt;
625+
AssertCounts(1, 3, 0, expected_nan_count, metrics);
626+
AssertCounts(2, 3, 0, expected_nan_count, metrics);
624627

625628
if (metrics.lower_bounds.contains(1)) {
626629
AssertBounds<float>(1, float32(), 1.2F, 5.6F, metrics);
@@ -658,8 +661,9 @@ void MetricsTestBase::ColumnBoundsWithNaNValueAtEnd() {
658661

659662
ASSERT_TRUE(metrics.row_count.has_value()) << "row_count should be set";
660663
EXPECT_EQ(*metrics.row_count, 3);
661-
AssertCounts(1, 3, 0, metrics);
662-
AssertCounts(2, 3, 0, metrics);
664+
auto expected_nan_count = ReportsNanCounts() ? std::optional<int64_t>(1) : std::nullopt;
665+
AssertCounts(1, 3, 0, expected_nan_count, metrics);
666+
AssertCounts(2, 3, 0, expected_nan_count, metrics);
663667

664668
if (metrics.lower_bounds.contains(1)) {
665669
AssertBounds<float>(1, float32(), 1.2F, 5.6F, metrics);

src/iceberg/test/metrics_test_base.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,9 @@ class MetricsTestBase {
6060
/// \brief Whether the format supports small row groups for testing
6161
virtual bool SupportsSmallRowGroups() const { return false; }
6262

63+
/// \brief Whether the format reports NaN counts for floating-point fields
64+
virtual bool ReportsNanCounts() const { return false; }
65+
6366
// Helper methods for assertions
6467
void AssertCounts(int field_id, std::optional<int64_t> expected_value_count,
6568
std::optional<int64_t> expected_null_count, const Metrics& metrics);

src/iceberg/test/parquet_metrics_test.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ class ParquetMetricsTest : public MetricsTestBase, public ::testing::Test {
106106
}
107107

108108
bool SupportsSmallRowGroups() const override { return true; }
109+
bool ReportsNanCounts() const override { return true; }
109110

110111
private:
111112
std::string temp_parquet_file_;

0 commit comments

Comments
 (0)