andygrove commented on code in PR #6851:
URL: https://github.com/apache/datafusion-comet/pull/6851#discussion_r4260583848
##########
spark/src/test/scala/org/apache/comet/CometIcebergWriteActionSuite.scala:
##########
@@ -2172,51 +2172,74 @@ class CometIcebergWriteActionSuite
}
}
- // https://github.com/apache/datafusion-comet/issues/6138. iceberg-rust
compares float partition
- // values with an equality that treats -0.0 and 0.0 as one value, so its
writers filed both under
- // whichever arrived first, and a read that pruned on the other value lost
rows. iceberg-java
- // keeps the two apart, so these writes fall back until iceberg-rust does
too.
- test("signed-zero float and double identity partitions match iceberg-java") {
+ // https://github.com/apache/datafusion-comet/issues/6138. iceberg-rust
compared float partition
+ // values with an equality that treated -0.0 and 0.0 as one value, so its
writers filed both
+ // under whichever arrived first, and a read that pruned on the other value
lost rows. Since
+ // apache/iceberg-rust#3327 it keeps them apart and puts every NaN in one
partition, as
+ // iceberg-java does. One shuffle partition puts every row in one task, so
the values meet in one
+ // writer. The fanout writer gets them interleaved. The clustered writer
gets each partition's
+ // rows together: Spark's sort ties -0.0 with 0.0, so a zero that repeats
could come back
+ // unclustered, which iceberg-java rejects too.
Review Comment:
Added in 4eae2874fe as "clustered writer rejects interleaved signed zeros
like the JVM writer". It hands the clustered writer `-0.0`, `0.0` and `-0.0` in
one task, and both writers have to fail with the same `IllegalStateException`,
at the same depth in the cause chain and with the same message, which names the
`f=-0.0/d=0.0` partition.
I used `failedWrites` and `clusteredWriterError` rather than
`assertFailsLikeSpark`, as the clustered-writer test for #5698 does.
`assertFailsLikeSpark` compares Spark's write with the split plan, but it
doesn't check that the native writer ran, and it only compares exception
classes. `failedWrites` fails unless the native write went through
`CometIcebergWriteExec`. The rows also skip Spark's sort, so the order the
writers see doesn't depend on how the sort orders the two tied zeros. Against a
libcomet built from main, the native write succeeds and the test fails.
--
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]
