andygrove commented on PR #5638: URL: https://github.com/apache/datafusion-comet/pull/5638#issuecomment-5530093620
_LLM-assisted: this reply and the changes it describes were written with Claude Code._ @jordepic filed upstream as promised: - apache/iceberg-rust#3141 — the `truncate` arithmetic - apache/iceberg-rust#3142 — `Year`/`Month` following the timezone tag Two things came out of writing them up that are worth correcting here, since both change what I said earlier on those threads. The `truncate_i64` overflow you and I were both treating as the headline is **debug-only**. In a release build Rust wraps and matches Java bit for bit, and both Comet CI and `make release` build `--release`, so it never affected a real write — only a debug or test build of a downstream writer. The genuinely broken one is `truncate_i32`, which uses `rem_euclid` and so diverges from Java *semantically* in release builds too, for any width above 2^30 with a positive value. Verified against `TruncateUtil` on JDK 17: | width | value | Java | iceberg-rust | | --- | --- | --- | --- | | 2000000000 | 1500000000 | -2000000000 | 0 | | 2147483647 | 2147483646 | -2147483647 | 0 | Iceberg Java only validates `width > 0`, so such a spec is legal. That is a wrong partition value, not a panic. The timezone one turned out to be more than the Comet-facing concern I described on the `temporal.rs` thread: iceberg-rust's `Year`/`Month` `transform` and `transform_literal` disagree with *each other* for a non-UTC-tagged array, because the literal path uses `DateTime::from_timestamp_micros` (UTC) while the array path uses `date_part`. So predicate projection and partition-value computation can diverge within iceberg-rust itself. Latent today since its own callers always pass `+00:00`-tagged arrays, but it does firm up the case for Comet keeping local kernels for those two. I have updated the test comment in `iceberg_write.rs` and the one in `CometIcebergSystemFunctionSuite` to point at the two issues instead of describing the workaround inline, which was your original ask. -- 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]
