fix: native Iceberg write panics on an evolved partition spec and on a timestamptz partition path

[andygrove]

Which issue does this PR close?

Closes#5691.
Closes#5694.
Closes#5693 (same root cause as #5691, one step further along).

Rationale for this change

Two native Iceberg write panics, both crossing the JNI boundary as a CometNativeException
rather than surfacing as an error Comet could fall back on or report cleanly.

#5691 / #5693 — evolved partition spec. iceberg-java's UpdatePartitionSpec keeps a dropped
partition field in a format-version-1 spec as a void transform so its field id survives, and
PartitionSpec#isUnpartitioned means "every field is void", not "no fields" — on the Rust side
too. The next write therefore routes through UnpartitionedWriter, which stamps every data file
with an empty partition struct, while ManifestWriter derives one partition summary per spec
field and zip_eqs the two:

itertools: .zip_eq() reached end of one iterator before the other
at <iceberg::spec::manifest::writer::ManifestWriter>::construct_partition_summaries

Dropping the void field's source column afterwards then broke the manifest's partition_type
resolution as well (No column with source column id 2 in schema), which is #5693.

#5694 — timestamptz partition path. iceberg-rust's microseconds_to_datetimetz takes
micros % 1_000_000 — negative for a pre-1970 value — casts it to u32 and multiplies by 1000,
then unwraps the None that DateTime::from_timestamp returns for the resulting out-of-range
nanosecond count. RandomData in Iceberg's own tests generates timestamps as
random.nextLong() % FIFTY_YEARS_IN_MICROS, so the value reaching the conversion is a legitimate
pre-epoch timestamp; the conversion is simply wrong for negative inputs. Still present on
iceberg-rust main, so there is nothing to bump the pin to.

What changes are included in this PR?

#5691 / #5693.encode_data_files_as_manifest now encodes the per-task transport manifest
against a field-less spec of the same spec id whenever is_unpartitioned() holds (a no-op in the
common already-field-less case), so the manifest's partition arity matches the empty partition
struct the writer produced. Nothing downstream loses information: the JVM re-reads this manifest
with the spec embedded in its own Avro metadata, then rebuilds each DataFile against the real
output spec, whose DataFiles.Builder drops partition data outright for an unpartitioned spec —
so the manifest that reaches storage carries exactly what iceberg-java's own writer would have
committed. Dropping the fields also skips the partition_type resolution that #5693 tripped over.

#5694. New CometLocationGenerator (native/core/src/execution/operators/iceberg_partition_path.rs)
replaces iceberg-rust's DefaultLocationGenerator. It lays files out identically but renders the
partition path itself, mirroring iceberg-java's PartitionSpec#partitionToPath. Per-field it
delegates to Transform::to_human_string and overrides only the arms where iceberg-rust disagrees
with iceberg-java:

Iceberg type	iceberg-java	iceberg-rust
timestamp	1969-12-31T23:59:58.5	1969-12-31 23:59:58.500
timestamptz	1969-12-31T23:59:58.5+00:00	panics on a negative value with a sub-second part; otherwise 1969-12-31 23:59:58.500 UTC
binary / fixed	base64	uppercase hex

I widened it past timestamptz because a function whose stated job is iceberg-java parity should
not knowingly leave two adjacent arms wrong, and Iceberg's own TestSparkDataFile SPEC (the
#5694 reproducer) partitions on binary and timestamp as well as timestamptz. The name/value
escaping is unchanged: form_urlencoded leaves exactly the byte set URLEncoder.encode(s, UTF_8)
leaves.

The generator resolves the partition type once per task instead of per file, which turns a spec it
could not render into an error at task start rather than a panic inside the infallible
LocationGenerator::generate_location.

Two qualifications, both documented in the module docs and in iceberg-writes.md:

• float and double still delegate, so a float partition directory reads f=1 where
  iceberg-java writes f=1.0. Matching Java means porting Float.toString/Double.toString — the
  same port Comet's cast(float as string) needs — which is much more than a partition directory
  name warrants, and unlike timestamptz it does not panic.
• "matches iceberg-java" means 1.8 or later. iceberg-java's own rendering changed in 1.8:
  1.5.2 (still the Spark 3.4 profile's pinned runtime) spelled timestamp/timestamptz
  directories with LocalDateTime.toString() / OffsetDateTime.toString()
  (ts=1969-12-31T23:59:58.500Z) and left the field name unescaped, both of which 1.8 changed.
  Comet emits the 1.8+ spelling on every profile rather than branching the renderer on the runtime
  version, since the directory name is cosmetic.

Distinct partition values still get distinct directories in all cases, and no reader parses these
names.

How are these changes tested?

New tests, all of which I confirmed fail against the pre-fix native library with the exact
panics from the issues (zip_eq() reached end of one iterator before the other for #5691;
attempt to multiply with overflow for #5694 — the debug-build face of the release-build
Option::unwrap() in the issue) and pass with it:

• CometIcebergWriteActionSuite, three end-to-end parity tests that write the same data twice,
  once natively and once through iceberg-java's writer, and compare:
  • timestamptz and binary partition paths match iceberg-java — the full set of committed
    partition directories must be equal between the two tables (on Iceberg 1.8+, per the
    qualification above), plus unconditional pinned expectations for the pre-epoch sub-second
    value, the epoch, and a microsecond-precision value.
  • writes after a V1 partition field is dropped match iceberg-java — walks the sequence from
    Native Iceberg write panics in construct_partition_summaries after partition spec evolution #5691 (ADD PARTITION FIELD → write → DROP PARTITION FIELD → write) and compares rows and
    the committed manifests' partition_spec_id / partition_summaries.
  • writes after a V1 partition source column is dropped match iceberg-java — the Native Iceberg write fails with "No column with source column id" after an identity partition field is dropped #5693 stage
    (DROP COLUMN → write), gated on Iceberg 1.11+. Earlier runtimes cannot commit this on
    either path: PartitionSpec.partitionType NPEs on the missing source before 1.10, and
    PartitionSpec.javaClasses still does on 1.10 (getResultType(null) returns null). 1.11
    substitutes UnknownType in both. The failure is in the driver-side commit the native path
    shares with the stock one, so it is an Iceberg-version limit rather than something this fix
    could remove.
• iceberg_write.rs: void_only_spec_write_round_trips_through_the_manifest,
  void_field_with_a_dropped_source_column_still_writes, and
  pre_epoch_timestamptz_partition_gets_a_java_shaped_directory drive the real iceberg-rust
  writer stack.
• iceberg_partition_path.rs: unit tests pinning each rendering against the strings the Java
  source produces, including ISO_LOCAL_DATE_TIME's always-print-seconds and
  strip-trailing-zeros-fraction behaviour (.5, not .500000), EXCEEDS_PAD year formatting,
  and timestamps past chrono's year-262143 calendar ceiling (which i64 micros can reach and
  iceberg-java has no ceiling for, hence the hand-rolled civil_from_days).

Full runs: the whole native workspace (cargo test --workspace) passes, and
CometIcebergWriteActionSuite passes on the spark-3.4 (Iceberg 1.5.2), spark-3.5 (1.8.1),
spark-4.0 (1.10.0) and spark-4.1 (1.11.0) profiles, as do the other Iceberg suites in the
scans group on 3.4 and 4.0.

Not verified here: Iceberg's own gradle suites (TestAlterTablePartitionFields,
TestSparkDataFile). Gradle cannot start in my environment — it fails binding its
FileLockContentionHandler socket. Two notes for reviewers on that:

1. The iceberg_spark_test_1.11 workflow will not catch these either, because
   dev/diffs/iceberg/1.11.0.diff does not set spark.comet.iceberg.write.enabled — the native
   write path is only reachable in those suites once test: enable the Iceberg split-operator and native write by default to surface test failures #5677 turns it on. That is why I put the
   equivalent iceberg-java-parity coverage in Comet's own suite instead.
2. The panic in Native Iceberg write panics generating the partition path for a timestamptz partition column #5694 is upstream in iceberg-rust, unfixed on main; this PR routes around it
   rather than fixing it there. Happy to file the upstream issue and add the reference here.

[sunchao]

Correctness

Reviewed d6a25964d36aef9a5ae8f6fd169325989f2d40ef against f97fb4519cad2a4f6fe435d66aa17eacbd919a9b. The native changes address two concrete failure paths: a V1 void-only spec retains fields while the unpartitioned writer produces an empty partition tuple, and upstream timestamp rendering can panic on a negative fractional epoch value. The temporary manifest now uses a fieldless spec with the original spec ID. The JVM still rebuilds data files against the actual output spec and commits through Iceberg, so this does not rewrite the table's partition history.

The location generator resolves the partition result types once, skips that resolution for effectively unpartitioned specs, and pairs fields and values by their common spec order. Source field IDs remain the basis of column projection; they are not treated as tuple positions. Null and void values remain null, timestamp rendering uses floor division for negative epochs, and binary/fixed values use base64 before URL encoding. Existing ordinal transforms retain their own rendering. Invalid partitioned schemas return an error while constructing the writer, before file creation.

There are two actionable P2 issues in the new regression tests, detailed inline: unconditional timestamp-path equality fails with Spark 3.4's Iceberg 1.5.2 formatter, and the post-source-column-drop scenario fails in the older JVM runtimes. These account for all four failed scan jobs in the supplied snapshot. The native fix does not remove those JVM-library limitations.

The native job passed all 15 added tests and all 1,182 executed tests, with four skipped. Both new JVM tests passed on Spark 4.1/Iceberg 1.11; both were canceled on Spark 4.2 because Iceberg was unavailable. All inspected jobs used merge d441ec8ee3badf561ef4239aa06a2ef028aafe4a, whose parents are the assigned base and head, and the reviewed paths match the head. An independent java.time oracle matched the extracted production formatter in 40,232 microsecond/nanosecond cases, including negative values and both integer extremes. This was a formatter check, not a local end-to-end write. Maintained Spark 3.5/4.0 timestamp and commit/abort sources were checked; maintained 3.4/4.1 sources were unavailable. At the supplied 2026-09-06T02:38:18.793Z cutoff, 61 checks succeeded, nine were skipped and four failed.

Performance

Caching the partition result type avoids resolving the schema for each generated file. Rendering and base64 encoding occur when creating file locations, rather than for every input row. Normalizing a void-only transport spec also avoids resolving columns that no longer exist. The writer's batching, rolling, fanout and clustered routing are unchanged. I found no additional performance issue requiring a change; no benchmark was run.

Design

The two changes fit the existing integration boundaries: manifest normalization belongs to the native-to-JVM transport, while path formatting belongs to the location generator. Retaining the original spec ID is essential for the JVM's output-spec lookup. Resolving fallible metadata during construction is appropriate because the location-generator interface itself cannot return an error. The existing JVM commit and abort flow remains authoritative. The requested test adjustments should retain void-only coverage across supported runtimes and separately qualify behavior that needs a newer Iceberg version.

Abstraction & complexity

The new module keeps path compatibility separate from writer orchestration and overrides timestamp and binary formatting while reusing other transforms. The standalone Gregorian conversion has a concrete reason: it supports the full signed microsecond range beyond Chrono's calendar limit. Its boundary tests and the independent oracle support that choice. I found no additional abstraction issue requiring a change. The documented path-parity claim should share the version qualification required by the first inline finding.

[sunchao]

Correctness

[P2] Make timestamp-path parity conditional on the Iceberg formatter version

This equality fails in the supported Spark 3.4 profile, which pins Iceberg 1.5.2. Its TransformUtil.humanTimestampWithZone uses OffsetDateTime.toString(), so the JVM writes 1969-12-31T23:59:58.500Z and 1970-01-01T00:00Z, while this patch writes 1969-12-31T23:59:58.5+00:00 and 1970-01-01T00:00:00+00:00. The Spark 3.4 scan job fails at this assertion with exactly those sets. Make the byte-for-byte JVM comparison version-aware, while retaining the pre-epoch write and readback checks on 3.4, and qualify the corresponding documentation claim. The older layout difference does not itself indicate incorrect stored values.

[andygrove]

You're right, and I'd missed that iceberg-java's own rendering changed here rather than just diverging from mine: 1.5.2's humanTimestampWithZone is ChronoUnit.MICROS.addTo(EPOCH, micros).toString(), and 1.8 moved it to DateTimeUtil.microsToIsoTimestamptz. While checking that I noticed 1.5.2 doesn't escape the partition field name either, which no Comet version has reproduced, so it's the same version boundary.

I've gated just the JVM comparison on icebergVersionAtLeast(1, 8) and left the pinned expectations and the readback unconditional, so 3.4 still covers the pre-epoch write that used to panic. I chose to emit the 1.8+ spelling on every profile rather than branch the renderer on the runtime version, since the directory name is cosmetic and nothing parses it back, and said so in the module docs and in the accepted-divergences list. Happy to make it follow the runtime instead if you'd rather Comet and the JVM writer always agree on 3.4.

[sunchao]

Correctness

[P2] Separate the dropped-source regression from runtimes that cannot commit it

The unconditional final stage fails in every Spark 3.4, 3.5 and 4.0 scan job. Their pinned Iceberg 1.5.2/1.8.1 runtimes fail resolving the dropped source in PartitionSpec.partitionType; 1.10.0 reaches commit but fails in PartitionSpec.javaClasses because the transform result type is null. The Spark 4.0 failure traces through SnapshotSummary and IcebergCommitExec, so normalizing the native transport manifest does not avoid this JVM failure. Iceberg 1.11 handles the missing source, and the new test passes in the Spark 4.1 job. Split the source-column-drop scenario behind the supported Iceberg-version condition while keeping the preceding void-only writes covered on older profiles; otherwise this regression test leaves four scan jobs failing.

[andygrove]

Agreed, and thanks for separating 1.10 out. I'd assumed the UnknownType fallback added to partitionType covered it, but javaClasses only got the same fallback in 1.11, so 1.10 gets further and still fails. Since that failure is in the driver-side commit the native path shares with the stock one, gating is the only honest option: normalising the transport manifest can't rescue it.

Split the source-column drop into its own test behind assume(icebergVersionAtLeast(1, 11)) and left the void-only stages unconditional, so #5691 itself stays covered on 3.4/3.5/4.0. Verified CometIcebergWriteActionSuite locally on spark-3.4 (Iceberg 1.5.2), spark-3.5 (1.8.1), spark-4.0 (1.10.0) and spark-4.1 (1.11.0), plus the other Iceberg suites in the scans group on 3.4 and 4.0.

[sunchao]

Correctness

Re-reviewed ce3379a92e9bf2f210d431207870f48109e89533 against 7f1e00189b1ed86f1cb5acd872d97fce694482b1. Both previous P2 findings are addressed. The timestamp test now gates only the byte-for-byte JVM directory comparison on Iceberg 1.8+, retaining the native pre-epoch write, fixed path expectations and readback through both readers on older versions. The documentation accurately qualifies the timestamp spelling and partition-name escaping differences.

The source-column-drop case is now a separate test requiring Iceberg 1.11+. The void-only case remains available on older runtimes and still checks native execution, one commit per write, full rows and committed partition summaries. The shared helper keeps the optional final schema-drop step explicit. Its version gate uses the loaded Iceberg library, rather than assuming a version from Spark.

The revision leaves the native formatter's executable content, manifest transport, JVM commit code and dependency pins unchanged. I rechecked the maintained Spark 3.5/4.0 epoch conversion and commit/abort semantics. There is no new execution-semantic change in this follow-up. Maintained Spark 3.4/4.1 sources remain unavailable locally.

Current CI confirms the intended coverage: the timestamp and void-only cases pass on Spark 3.4, 3.5 and both Spark 4.0 scan jobs. The source-drop case is explicitly canceled there by the version condition, and all three pass on Spark 4.1. The previously failing Spark 3.4 job now passes. Spark 4.2 cancels all three because its Iceberg runtime is unavailable. The native job passes 1,198 tests with four skipped, including all 15 tests added by this PR. All seven inspected jobs checked out 37702c7f82d999d7570376b10952c1aa5d87104a, whose parents are the assigned base/head and whose entire tree equals the head. At the 2026-09-08T01:17:36.367Z snapshot, 64 checks succeeded and nine were skipped. No local product tests were rerun. I found no remaining or new P1/P2 issue.

Performance

The follow-up adds version checks only in tests and changes documentation. It introduces no new per-row or per-file work, and the runtime dependency set is unchanged. No new performance finding or benchmark result is claimed.

Design

Separating the two schema-evolution cases preserves coverage of the native fix on older runtimes while making the JVM limitation explicit. Keeping one documented directory spelling also avoids adding runtime-version branching to the writer. These changes address the review without broadening the production implementation.

Abstraction & complexity

The shared evolution helper removes duplicated setup and exposes its single variation through dropSourceColumn. The existing version helper is reused. I found no new unnecessary abstraction or complexity in the revision.