andygrove opened a new issue, #5836:
URL: https://github.com/apache/datafusion-comet/issues/5836
### 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.
--
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]
