apache / apache/datafusion-comet

Native Iceberg write renders float/double partition values differently from iceberg-java, and fails with "File name too long" for large values

Open
#5,836 0 comments 0 reactions 1 assignee Claimed by @andygrove View on GitHub
area:Iceberg area:writer bug requires-triage
Dominant language
Scala
Stars
1.3k
Forks
373
Avg merge
2d 4h
Merged PRs (30d)
198

Description

### Describe the bug

An identity partition on a `double` or `float` column produces a partition directory name that does not match iceberg-java's, and for values outside `[1e-3, 1e7)` the name is long enough that the write fails outright.

iceberg-java renders the value with `Double.toString`, which switches to scientific notation outside that range, so `Double.MAX_VALUE` becomes the 22-character `d=1.7976931348623157E308`. Comet's `human_string` delegates float and double to iceberg-rust's `Transform::to_human_string`, which uses Rust's own `Display`, and that never uses an exponent: the same value becomes a 309-character directory name. Linux caps a single path component at 255 bytes, so the write dies in the Parquet writer:

```
org.apache.comet.CometNativeException: Unexpected => Failed to finish parquet writer.,
source: External: External: Unexpected => Failure in doing io operation,
source: Unexpected (persistent) at write, context: { service: fs,
path: .../data/b=false/i_bucket=null/l=4882527599540352535/f=-0.7821834/d=1797693134862315700000000...0000/date=2056-09-14/... }
=> invalid filename, source: File name too long (os error 36)
```

`Double.MIN_VALUE` hits the same wall from the other end: iceberg-java writes `4.9E-324`, the native writer writes `0.000...005` with 324 digits.

Below the length limit the write succeeds but the directory name still differs from what iceberg-java would have produced: `1.0` becomes `1`, `1.0E20` becomes `100000000000000000000`. Nothing parses the partition path, so that part is cosmetic.

The divergence is already documented in `native/core/src/execution/operators/iceberg_partition_path.rs`, where float and double are deliberately left delegating on the grounds that, unlike `timestamptz`, they do not panic. That reasoning holds for the value itself but not for the path length, which is a hard failure.

### Steps to reproduce

Spark 4.1.3, Iceberg 1.11.0, with `spark.comet.iceberg.write.enabled=true` and the Iceberg Spark SQL test setup from `dev/diffs/iceberg/1.11.0.diff`:

```
./gradlew -DsparkVersions=4.1 -DscalaVersion=2.13 -DflinkVersions= -DkafkaVersions= \
:iceberg-spark:iceberg-spark-4.1_2.13:test --tests '*TestSparkDataFile*' -Pquick=true -x javadoc
```

`testValueConversionWithEmptyStats` and `testValueConversionPartitionedTable` both fail, on Iceberg 1.8, 1.9, 1.10 and 1.11. Both write a table partitioned by, among other columns, an identity `double` holding `Double.MAX_VALUE` and `Double.MIN_VALUE`.

### Expected behavior

The partition directory for a `float` or `double` value is the string iceberg-java's `Transform#toHumanString` would produce, so the same table written through either writer produces the same layout and the path never exceeds the filesystem's component limit.

### Additional context

Found by turning the two Iceberg write flags on by default in #5677.

These two cases previously failed on the `Option::unwrap` panic in #5694; fixing that in #5729 moved them on to this failure.

Java's `Double.toString` shortest-repr algorithm is the same one Comet's `cast(double as string)` needs, so a port would serve both. A narrower fix that only avoids the hard failure would still leave the directory names diverging.

Iceberg deprecated float and double partitioning in 1.3, so this affects existing tables rather than new ones.

Part of #5649.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.