Skip to content

Native Iceberg writer counts NaNs under NULL structs and NULL list or map entries #6562

Description

@andygrove

Describe the bug

The native Iceberg writer counts NaNs that sit under a NULL parent in a data file's nan_value_counts. Iceberg Java never sees those values, because a NULL struct, list or map writes nothing for its children, so the native manifest can record more NaNs for a field than the JVM writer does.

The NaN counter is iceberg-rust's NanValueCountVisitor (crates/iceberg/src/arrow/nan_val_cnt_visitor.rs at Comet's pinned rev bb1e4a4). It counts each float child array with only that child's own validity. A struct child's value at a NULL struct slot is counted, and so are the elements that a NULL list or map entry still points at. Arrow allows both layouts. nullif produces them, because it replaces the top-level validity buffer and leaves the children and offsets as they were.

A Comet plan produces them from IF(cond, col, NULL) or CASE WHEN cond THEN col END over a nested column. For nested types Comet's CaseWhenExpr evaluates through DataFusion's CaseExpr, which takes its InfallibleExprOrNull path when the THEN branch is a column, and that path is nullif(col, NOT cond).

Steps to reproduce

Write 100 rows where every third value is NaN to Parquet, then insert them through a projection that nulls the odd rows:

CREATE TABLE t (id INT, v DOUBLE, s STRUCT<x: DOUBLE>, xs ARRAY<DOUBLE>) USING iceberg;

-- src is a Parquet file with v = IF(id % 3 = 0, NaN, id), s = named_struct('x', v), xs = array(v)
INSERT INTO t
SELECT id, IF(id % 2 = 0, v, NULL), IF(id % 2 = 0, s, NULL), IF(id % 2 = 0, xs, NULL)
FROM src;

SELECT nan_value_counts FROM t.data_files;

The native plan is CometIcebergWrite <- CometProject <- CometNativeScan. I compared it with the same insert through Iceberg Java:

Field 3.5 / Iceberg 1.8.1, native JVM 4.1 / Iceberg 1.11.0, native JVM
v 17 17 17 17
s.x 34 17 34 17
xs.element 34 17 none none

The top-level column is right, because nullif sets the double array's own validity. The struct field is wrong on every profile: from Iceberg 1.10, ParquetMetrics drops the metrics of fields under a list or map but keeps those of struct fields, and Comet passes the native NaN count through. The list element is wrong only where nested metrics are kept, Iceberg before 1.10 (the Spark 3.4 and 3.5 profiles). Map values go through the same path as list elements.

Both tables return the same rows, and the Parquet data is right, since the Arrow writer honors the parent's validity. Only the metric is wrong. Spark can push predicates onto struct fields, though, and a NaN count that reaches the field's value count would make Iceberg's metrics evaluators treat the file as holding nothing but NaNs. I haven't built a case where that changes a query result.

Expected behavior

nan_value_counts matches Iceberg Java: a value under a NULL struct, list or map is not counted.

Additional context

This is the same visitor as #6146, where it counts the list and map elements outside an already-sliced batch. #6238 fixes that by gathering such a batch with take, but that doesn't cover this:

  • A batch whose children span its rows, as in the reproduction, is handed on without a gather.
  • arrow 59.3's take drops the elements of a NULL list or map entry at the level it gathers. It keeps a NULL struct slot's child values, though, since it takes every child at every index. It also keeps the elements under NULL entries of a list nested inside the gathered one, which MutableArrayData copies as whole ranges.

A Comet-side fix could push each struct's validity into its children and drop the ranges of NULL list and map entries before the writer counts, recursively. Alternatively the visitor could honor the parent's validity and offsets upstream, which would also fix #6146 and let both workarounds go.

The same 4.1 run also shows a smaller, separate difference: value_counts and null_value_counts for s.x are 100 and 50 natively but 50 and 0 through the JVM. This looks like Iceberg Java 1.10+ taking a float field's metrics from its value writers, which skip a field under a NULL struct, while the Parquet footer counts it as a null. The native writer and Iceberg 1.8.1 both report the footer's counts.

Part of #5649. Found while addressing review feedback on #6238.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

area:Icebergarea:writerNative Parquet writerbugSomething isn't workingpriority:mediumFunctional bugs, performance regressions, broken features

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions