Gabriel39 commented on PR #68128:
URL: https://github.com/apache/doris/pull/68128#issuecomment-5773880937
Follow-up suggestion: populate writer-side `nan_value_counts` so this
correctness fix does not unnecessarily disable file pruning for Doris-written
files.
The new `OR isNaN(col)` branch must retain a file when its NaN count is
unknown. A file containing only {1.0, 2.0, 3.0} cannot be ruled out for `d >
100 OR isNaN(d)` if its NaN count is absent, whereas an explicit zero lets
Iceberg prune it safely.
The transport field already exists: `TIcebergColumnStats.nan_value_counts`
in `DataSinks.thrift`. The missing pieces are producing it in BE and consuming
it in FE:
1. Accumulate per-file NaN counts for the actual FLOAT/DOUBLE values
written, keyed by Iceberg field ID. A shared collector at the
`VIcebergPartitionWriter` layer is one possible implementation for both Parquet
and ORC. Ignore NULL payloads, count NaNs rather than infinities, keep counts
separate across file rollover, and report an explicit zero only after the field
has been fully counted. Do not infer zero from min/max. An initial
implementation can cover top-level floating fields and leave unsupported nested
fields unknown.
2. Attach the counts to the file's commit statistics.
`IcebergWriterHelper.buildDataFileMetrics()` currently passes `null` as the
NaN-count argument to `Metrics`; read the supplied map, apply the table's
metrics policy, and pass it through. Preserve missing counts as unknown for
older BEs or unsupported paths; never replace missing entries with zero. No new
Thrift field is needed.
3. Add Parquet/ORC tests that verify the actual manifest counts and pruning:
a finite-only file reports zero and is pruned by an out-of-range predicate; a
file with NaN reports the correct positive count and remains a candidate. Also
cover NULL/infinity, multiple batches, file rollover, disabled metrics, and an
absent count from an older producer. Check query results as well as planned
splits.
This can be a separate performance follow-up; the correctness fix in this PR
should remain. Newly written files would benefit, while historical files with
missing counts would continue to require conservative pruning until their
metrics are rebuilt from the data or the files are rewritten.
--
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]
