feat: bump iceberg-rust and write float and double identity partitions natively

[andygrove]

Which issue does this PR close?

No issue. This lifts one of the restrictions listed in #5643: the fallback that #6531 added for #6138.

Rationale for this change

Since #6531, a native Iceberg write falls back to iceberg-java when the output partition spec has a float or double field. iceberg-rust compared float partition values with OrderedFloat's equality, which treats -0.0 and 0.0 as one value. Both of the native writer's grouping paths then filed rows with either value under whichever arrived first, and a read that pruned on the other value lost rows (#6138).

apache/iceberg-rust#3327 gives PrimitiveLiteral hand-written PartialEq and PartialOrd that follow Java's Float.compare and Double.compare: -0.0 sorts before 0.0, and all NaNs are equal. That is how iceberg-java compares partition keys, so with the pin past it the fallback can go.

What changes are included in this PR?

• The iceberg and iceberg-storage-opendal pin moves from af1da4c to 1f3bc34, the merge commit of fix(spec): treat -0.0 and 0.0 as distinct float literals iceberg-rust#3327. That is 17 commits. Cargo.lock picks up apache-avro 0.22 and nine new crate versions (ouroboros, strum 0.28, aliasable and their helpers).
• CometIcebergNativeWrite drops the float and double partition rule, and IcebergReflection drops its helper.
• Two other upstream changes in the range need code changes:
  • fix(scan): validate deletion vector delete tasks iceberg-rust#3145 makes FileScanTaskDeleteFile's fields private behind a builder that validates on build(), and holds the deletion-vector offset and size as u64. The planner builds delete files through the builder and rejects a negative record count, offset or size from the proto. IcebergScanExec::fill_delete_file_sizes sizes a delete file by rebuilding it, as it already does for the task. The JVM serde already sends every field the builder requires for a deletion vector.
  • fix(transform): floor day transform for pre-epoch timestamps iceberg-rust#3323 fixes iceberg-rust's day for pre-epoch timestamps (day transform puts some pre-epoch timestamps in the next day iceberg-rust#3315). Comet computes the time-transform partition values of timestamps itself, so nothing it writes changes. The canary test that pins iceberg-rust's values now expects the fixed day. iceberg-rust still differs from iceberg-java for a timestamp exactly 999999 microseconds into a unit (Native Iceberg years/months/days/hours differ from Iceberg for some pre-1970 timestamps #6426), so Comet keeps computing these values.
• Docs: the user guide no longer lists the fallback. The contributor guide and the Iceberg write review skill keep Native Iceberg write merges -0.0 and 0.0 rows into one float/double identity partition #6138 as an example of iceberg-rust's value semantics, and now note the upstream fix.

The rest of the range needs no Comet changes:

• apache-avro 0.22 (perf(manifest): upgrade to apache-avro 0.22 and read manifest entries without Value iceberg-rust#3354). When two partition fields share a decimal type, iceberg-rust used to define its named Avro type, such as decimal_10_2, once for each field. Now it defines the type once and refers to it after that. A new test checks that iceberg-java still reads the native manifests.
• Page-index pruning for FIXED_LEN_BYTE_ARRAY decimals (feat(reader): page-index pruning for FIXED_LEN_BYTE_ARRAY decimals iceberg-rust#3328). Comet doesn't push residual predicates on decimal columns to iceberg-rust (IcebergReflection.pageIndexUnsupportedColumns), so this doesn't reach the native scan yet.
• Typed NULLs for nested columns added by schema evolution, and an error for a missing field with a non-primitive initial default (fix: fill schema-added nested columns with typed NULL arrays on read iceberg-rust#2635). CometScanRule already falls back for V3 columns with defaults.
• An ADLS path fix for deletes (fix(storage-azdls): drop the leading slash from relative paths iceberg-rust#3311). Comet doesn't write to ADLS.
• REST catalog sessions, which Comet doesn't use, and dependency and CI bumps.

The nested schema evolution guard from #6504 stays, because it waits on apache/iceberg-rust#3255, which is still open.

How are these changes tested?

• CometIcebergWriteActionSuite "signed-zero and NaN float and double identity partitions match iceberg-java" requires the native writer to run.
  • Every row goes through 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, and a clustered write that returns to an earlier zero fails on iceberg-java too.
  • It compares three things against iceberg-java: the partition directories, the committed partition values with their record counts, and the rows that a pruned read returns for each zero and for NaN.
  • Against a libcomet built from main with the fallback removed, both writers put every zero under f=-0.0/d=0.0, so the test fails. With this pin it passes.
• "two decimal identity partitions of the same type match iceberg-java" writes decimal(10, 2) and decimal(38, 2) pairs natively and compares them with iceberg-java.
• "float and double partition paths match iceberg-java" now checks the native writer's directories.
• CometIcebergWriteDetectionSuite replaces four fallback tests with one that checks a double identity partition is Compatible.
• New Rust tests cover the planner's negative-coordinate checks and the scan's delete-file rebuild.
• Locally, on the default Spark 4.1 profile (Iceberg 1.11), all of these pass:
  • all 718 datafusion-comet lib tests;
  • clippy, with the same flags as CI;
  • the 14 Iceberg JVM suites that don't need MinIO: 420 passed, 0 failed, and 1 canceled (the unknown-transform test, which is canceled on this profile on main too);
  • the strict-warnings compile on Spark 3.5.
• On Spark 3.5 (Iceberg 1.8.1), the changed write and detection tests pass.

A pin bump can change file bytes and manifests, and the other Spark profiles pin Iceberg 1.5.2, 1.8.1 and 1.10.0. So this PR is labelled run-iceberg-tests and run-all-spark-profiles.

[sunchao]

Reviewed the entire 12-file diff from bfacc43a50aba13da4fa8a828db52309541191b4 to 537488876ef45fe718900e3da9d4a88376456f77. The PR is not a draft. The snapshot and refreshed discussion contained no existing reviews or comments. One introduced P2 issue was identified.

Routed skills: review-comet-pr, review-comet-iceberg-write-pr, and review-comet-expression-pr for the serde eligibility change. Read AGENTS.md and the relevant contributor guidance.

Summary

• Prior state and problem: The base falls back to iceberg-java for float/double partitions because iceberg-rust merged signed zeros, potentially losing rows during partition pruning.
• Design approach: Advance iceberg-rust through its signed-zero fix, remove the fallback, and adapt delete-file construction to the new validated builder API.
• Correctness: The upstream equality now distinguishes signed zeros while treating NaN payloads alike. Both Comet partition-splitting paths use that equality. Native writer probes and Java manifest readers preserved the expected partitions and record counts. No introduced runtime correctness issue meeting the P1/P2 bar was identified.
• Compatibility analysis: Checked Spark SQLOrderingUtil and write-ordering sources for 3.4.3, 3.5.9, 4.0.4, 4.1.3, and 4.2.0, plus iceberg-java comparators for 1.5.2, 1.8.1, 1.10.0, and 1.11.0. Spark ties signed zeros when sorting, whereas Iceberg partition keys distinguish them. The tests account for that distinction. The new detection-test tuples do fail the Scala 2.12 strict build, as detailed in the finding.
• Key design decisions: Keeping Comet’s timestamp partition kernels is appropriate because the upstream day fix does not resolve every Java/Rust timestamp difference. The remaining eligibility guards and Java metric reconstruction remain intact.
• Implementation sketch: The Scala change removes the floating-point rule and reflection helper. Rust converts deletion-vector coordinates with checked unsigned conversions and rebuilds sized delete files while forwarding every other field. Tests cover builder rejection and field preservation.
• Performance: Delete-file stat requests remain deduplicated, and Puffin files still avoid unnecessary HEAD requests. The builder migration retains comparable per-file copying. No evidence-backed P1/P2 performance regression was identified. No benchmark was run or speedup quantified.
• Design: Fixing value semantics upstream and removing the temporary gate is straightforward. Reviewing the pin as a behavioral change also covered manifest serialization, nested null arrays, decimal page pruning, temporal transforms, and Azure path handling.
• Abstraction & complexity: sized_delete_file centralizes the required builder plumbing without introducing another grouping mechanism. Its preservation test makes the migration easier to verify. No additional complexity issue met the reporting bar.
• Behavioral changes worth calling out: Float/double identity partitions become native again relative to the base. Compared with branch-1.1, which admitted these writes using the older equality, the signed-zero separation is an intended correctness improvement. Malformed deletion-vector coordinates now fail during planning. Comet’s timestamp write calculations remain unchanged.
• Suggested improvements: Construct the three detection-test entries as explicit nested tuples so Scala 2.12 compiles them under strict-warnings. No additional P1/P2 improvement was identified.

Exact-head CI: Strict Scala warnings (Spark 3.5, JDK 17) failed on the three new tuple expressions. At the final inspection, the head had 21 successful checks, one failure, seven running checks, and 44 skipped checks. Native builds and the labeled Iceberg/profile runs had not produced complete test verdicts. Both run-iceberg-tests and run-all-spark-profiles are present.

Validation: 161 focused native Iceberg tests and the deletion-vector planner test passed. A disposable real-writer probe exercised both writer modes, top-level and nested float/double fields, signed zeros, different NaN payloads, nulls, infinities, and batch boundaries. All four pinned iceberg-java readers decoded its four manifests with the expected partition bit patterns and record counts. The Scala compiler reproduction failed for the added syntax and passed with explicit nested tuples. Temporary test changes were removed and the checkout is clean.

Limits: Full JVM suites, upstream Spark/Iceberg SQL suites, and object-store integration were not run locally. Pending CI remains a validation limitation. Local native compilation required temporary JNI headers for the installed Java runtime.

[sunchao]

[P2] Construct these entries as explicit nested tuples. The three new "part_*" -> (...) expressions trigger Scala 2.12’s adapted-argument warning because -> accepts one argument and the compiler implicitly creates a three-tuple. The supported Spark 3.5 strict-warnings build promotes this warning to an error, so test-compile fails before tests can run. Using ("part_float", ("v FLOAT", "v", "CAST(1.5 AS FLOAT)")), and the equivalent form for the other entries, avoids the adaptation and restores compilation.

Evidence: Exact-head CI job https://github.com/apache/datafusion-comet/actions/runs/38055061563/job/114222370917 fails at lines 1509–1511 with Adapting argument list by creating a 3-tuple while running ./mvnw -B test-compile -Pspark-3.5 -Pstrict-warnings -DskipTests. Independently compiling the three entries with Scala 2.12.18 and -Xlint:adapted-args -Xfatal-warnings exited 1. The explicit nested-tuple version exited 0.

[andygrove]

Thanks. Fixed in 1036ae5, and 3ffae5a then switched this test to the suite's createTable table, so the tuples are gone. Strict Scala warnings (Spark 3.5, JDK 17) passes on 3ffae5a.