github-actions[bot] commented on code in PR #68128:
URL: https://github.com/apache/doris/pull/68128#discussion_r4079063994


##########
be/src/format/transformer/vparquet_transformer.cpp:
##########
@@ -262,10 +272,77 @@ Status VParquetTransformer::_parse_schema() {
     return Status::OK();
 }
 
+namespace {
+
+// Branchless so the compiler can vectorize it: this runs over every floating 
value written.
+template <typename Container>
+int64_t count_nan_values(const Container& data, const NullMap* null_map) {
+    int64_t nan_count = 0;
+    const size_t rows = data.size();
+    if (null_map == nullptr) {
+        for (size_t i = 0; i < rows; ++i) {
+            nan_count += static_cast<int64_t>(std::isnan(data[i]));
+        }
+        return nan_count;
+    }
+    for (size_t i = 0; i < rows; ++i) {
+        nan_count += static_cast<int64_t>((*null_map)[i] == 0 && 
std::isnan(data[i]));
+    }
+    return nan_count;
+}
+
+} // namespace
+
+// Iceberg excludes NaN from a column's bounds by spec ("NaNs are not 
permitted as lower or upper
+// bounds"), so nan_value_counts is the ONLY metadata that can prove a file 
holds no NaN. Without it
+// InclusiveMetricsEvaluator.isNaN must assume NaN may be present and a float 
range predicate cannot
+// prune the file at all (see the FLOAT/DOUBLE leaves in FE 
IcebergPredicateConverter).
+//
+// A field listed here is a claim that every one of its values was counted, 
which is what makes a
+// reported zero trustworthy. Only top-level FLOAT/DOUBLE columns qualify: a 
floating field nested in
+// a struct/list/map is not a block column of its own, so it is left out of 
the map entirely and stays
+// "unknown" -- conservative and unprunable, rather than wrongly claimed 
NaN-free.
+void VParquetTransformer::_init_nan_value_counts() {
+    if (!_collect_column_stats) {
+        return;
+    }
+    const auto& columns = _iceberg_schema->columns();
+    for (size_t i = 0; i < columns.size(); ++i) {

Review Comment:
   [P2] Skip NaN scans for metrics-disabled columns
   
   `collect_column_stats` is table-wide. With 
`write.metadata.metrics.default=none`, one non-floating field overridden to 
`full`, and many FLOAT/DOUBLE fields left disabled, `shouldCollectColumnStats` 
still sends `true`, so this loop registers every floating field and 
`_count_nan_values` adds a full `isnan` pass over each one on every block. 
`IcebergWriterHelper.filterLogicalMetrics` then drops those disabled field IDs, 
meaning the added O(rows * disabled-float-columns) memory traffic can never 
affect the manifest. Please thread the eligible floating field IDs (or 
equivalent per-field policy) to BE and count only fields whose effective 
metrics mode is not `none`; a mixed-mode test should pin that disabled floating 
columns are not scanned.



-- 
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]

Reply via email to