github-actions[bot] commented on code in PR #65493:
URL: https://github.com/apache/doris/pull/65493#discussion_r3565810327
##########
be/src/format_v2/parquet/parquet_statistics.cpp:
##########
@@ -118,17 +120,29 @@ bool set_decoded_field(const ParquetColumnSchema&
column_schema, DecodedValueKin
return read_decoded_field(column_schema, view, field, timezone).ok();
}
+template <typename NativeType>
+bool valid_min_max(const NativeType& min_value, const NativeType& max_value) {
+ if constexpr (std::is_floating_point_v<NativeType>) {
+ // Parquet requires readers to ignore min/max statistics if either
bound is NaN.
+ return !std::isnan(min_value) && !std::isnan(max_value);
+ }
+ return true;
+}
+
template <typename ParquetDType>
bool set_decoded_min_max(const std::shared_ptr<::parquet::Statistics>&
statistics,
const ParquetColumnSchema& column_schema,
DecodedValueKind value_kind,
ParquetColumnStatistics* column_statistics,
const cctz::time_zone* timezone) {
auto typed_statistics =
std::static_pointer_cast<::parquet::TypedStatistics<ParquetDType>>(statistics);
- if (!set_decoded_field(column_schema, value_kind, typed_statistics->min(),
- &column_statistics->min_value, timezone) ||
- !set_decoded_field(column_schema, value_kind, typed_statistics->max(),
- &column_statistics->max_value, timezone)) {
+ const auto& min_value = typed_statistics->min();
+ const auto& max_value = typed_statistics->max();
+ if (!valid_min_max(min_value, max_value) ||
+ !set_decoded_field(column_schema, value_kind, min_value,
&column_statistics->min_value,
Review Comment:
This still drops the valid null-count path when the only invalid part is the
NaN min/max. `TransformColumnStatistics` fills `has_null_count`, `has_null`,
and `has_not_null` before this check, but when `valid_min_max` fails the result
reaches `make_zonemap_from_statistics` with `has_not_null=true` and
`has_min_max=false`; that helper returns `nullptr` in that shape, so
`eval_null_zonemap` treats even `f IS NULL` as unsupported. The page-index NaN
path below has the same shape after returning success with `has_min_max=false`.
For an all-non-null float row group/page with NaN bounds and `null_count=0`, we
now keep it for `f IS NULL` even though the null count is still valid. Can we
carry a pass-all/range-unusable zonemap that preserves
`has_null`/`has_not_null`, so range predicates stay conservative while null
predicates still use the null-count metadata?
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
