Skip to content

fix: read Iceberg tables partitioned by an unknown transform - #5759

Merged
andygrove merged 2 commits into
apache:mainfrom
andygrove:shady-arbor-0ddb89bf
Sep 8, 2026
Merged

fix: read Iceberg tables partitioned by an unknown transform#5759
andygrove merged 2 commits into
apache:mainfrom
andygrove:shady-arbor-0ddb89bf

Conversation

@andygrove

Copy link
Copy Markdown
Member

Which issue does this PR close?

Closes#5758.

Rationale for this change

Iceberg CI is red on main: TestForwardCompatibility.testSparkCanReadUnknownTransform fails on the Iceberg 1.8.1 legs with

org.apache.comet.CometNativeException: General execution error with reason:
Invalid Iceberg scan task: DataInvalid => Non-empty FileScanTask partition requires a partition spec.

The table is partitioned by a zero transform that no Iceberg release defines, standing in for one written by a newer Iceberg. Reading it is supposed to work — that is Iceberg's forward-compatibility contract, and every data column is present in the file.

Iceberg Java parses an unrecognized transform into an UnknownTransform whose toString() is the original name, and whose getResultType() is StringType, notUnknownType. So the "drop partition fields with an unknown type" filter in serializePartitionData does not drop the field, and the spec reaches native as "transform":"zero" with a real partition value beside it. serde_json::from_str::<PartitionSpec> rejects zero, leaving the task with partition = Some(..) and partition_spec = None — the one pair iceberg-rust's FileScanTask::validate() rejects.

That validation is new: #5262 bumped iceberg-rust to a revision where FileScanTask's fields are private and construction goes through a validating builder. Before it, the task was built by struct literal and nothing in this query's read path consumed the missing spec, so it happened to work.

Transform::Unknown is iceberg-rust's model of the same situation and is conservative in the right direction — no partition constants, no pruning — and its result type is string, matching UnknownTransform.getResultType, so the partition type Comet already serializes alongside the spec stays in agreement with it.

What changes are included in this PR?

  • IcebergReflection.Transforms.forNative maps a transform name that iceberg-rust's Transform::from_str would reject onto unknown. CometIcebergNativeScan runs the spec's transform names through it when building the spec JSON.

    This is safe because the transform name reaches nothing in the native read but the identity test that builds the partition constants map (_spec_id uses the spec id; _partition matches partition values by field id). identity is matched exactly and so is never rewritten, and every other transform contributes no constants either way.

  • Corrected the comments on the native side that described the old (mistaken) understanding — that an unknown transform's field is filtered out Scala-side as an unknown type, and that the spec-id-from-JSON fallback exists for forward-compatibility tables. That fallback is now purely defensive.

The write side needs no equivalent change: Iceberg Java refuses to write through an unknown transform before Comet is involved, which testSparkWriteFailsUnknownTransform asserts.

How are these changes tested?

Three layers, plus the Iceberg Java suites this fixes.

End-to-end (CometIcebergNativeSuite, new test): writes an identity-partitioned table, rewrites its spec's transform to zero, and reads it back through the native scan. With the fix reverted it fails with the CI message verbatim; with the fix it passes on spark-3.4/Scala 2.12 (Iceberg 1.5.2), spark-3.5 (1.8.1), and spark-4.0 (1.10.0).

It is assume-skipped on 4.1, where Spark validates a V2 relation's metadata columns on every read — forcing SparkTable.metadataColumns() -> Partitioning.partitionType(), which rejects an unknown transform in the analyzer with or without Comet. Iceberg disabled its own copy of the test there for the same reason (SPARK-55626), which is also why only the 1.8.1 legs are failing.

Native (planner.rs, new test): pins both directions — a spec carrying unknown plus partition data builds a task that keeps its spec, its Transform::Unknown, its partition value and a string-typed unified partition type; the same input spelled zero still fails with the reported error.

Scala (IcebergReflectionSuite, two new tests): checks forNative against real Iceberg Transform objects. The eight transforms iceberg-rust understands round-trip unchanged (the round-trip assertion matters as much as the answer — forNative matches on Transform.toString, so a version that renders one differently from its JSON spelling would silently start rewriting it), zero rewrites to unknown, and partitionType() for it is string. Green on Iceberg 1.5.2, 1.8.1 and 1.11.0.

Also run: full CometIcebergNativeSuite (100 on 4.1, 95 on 3.5), CometIcebergNativeScanSuite + CometIcebergWriteDetectionSuite + CometIcebergRewriteActionSuite (60), all 275 native core tests, and clippy/rustfmt/spotless/scalastyle.

Iceberg Java surfaces a transform it does not recognize as an
UnknownTransform whose toString is the original name (e.g. "zero") and
whose result type is string, not unknown -- so serializePartitionData's
unknown-type filter keeps the field and the spec reaches native with a
transform name iceberg-rust cannot deserialize, alongside a real
partition value. The task then has partition values and no spec, which
FileScanTask's validation rejects.
Map such a name onto "unknown", which iceberg-rust deserializes into
Transform::Unknown -- its model of the same thing, with the same string
result type Iceberg Java reports, so the partition type serialized
alongside the spec still agrees with it. Safe because the transform name
reaches nothing in the native read but the identity test that builds the
partition constants map, and identity is matched exactly.

@rich7420rich7420 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, LGTM
@andygrove thanks for the pathc!

@sunchaosunchao left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correctness

Reviewed ab17ac7c against 7f1e0018. No verified P1/P2 findings. Previously, an Iceberg Java UnknownTransform such as zero kept its string partition value, but its name failed native partition-spec parsing. The nonempty partition then reached task validation without a spec and failed. The new serialization helper maps unrecognized names to the native unknown placeholder while preserving supported transforms, spec/field IDs, partition values and their alignment.

This matches Iceberg Java's conservative read behavior: unknown transforms have a string result type and cannot project partition predicates. In the pinned native reader, only identity transforms supply source-column constants; unknown therefore reads data columns from the file. The existing schema-history augmentation also provides partition source IDs when columns were projected out or dropped. The change does not alter expression evaluation, null conversion, overflow handling or write admission.

The maintained Spark 3.5 and 4.0 branches expose metadata columns lazily through DataSourceV2Relation, consistent with the test's bare path read. The regression compares the result with Comet disabled and separately requires a native Iceberg scan. This qualifies ordinary reads; it does not establish new support for queries that ask Iceberg Java to build metadata-column types for an unknown transform. Native _spec_id and _partition handling preserve IDs and values without applying that transform.

Validation

Verified CI's actual checkout 4f4aa3f6, its exact base/head parents, and its entire tree against this head. The native job passed 1,184 tests, with four skipped, including the new unknown-transform task test and its rejection control for the original name. The new native-read regression and both transform-helper tests passed on Linux Spark 3.4, Linux Spark 3.5, Linux Spark 4.0, and macOS Spark 4.0. Their native artifact IDs and digests match the inspected producers. The Spark 4.1 job passed the helper tests but canceled the read regression under its explicit version guard.

The snapshot contains 76 successful and six skipped checks. I did not run a local Comet build or Spark suite. Maintained Spark 3.4/4.1 source branches were unavailable; Spark 3.4 CI execution is separate evidence, and Spark 4.1 read support is not demonstrated. The author's reverted-fix comparison was not independently rerun.

Performance

Normalization adds a set lookup and, for parameterized or unrecognized names, a regex match during task serialization. It runs per partition field per file task before the existing JSON pool deduplication, so it is not strictly once per unique spec. It adds no per-row evaluation or data copy. The matcher is compiled once, and the work is small beside the existing reflection and JSON serialization. I found no material new cost requiring a benchmark; no performance gain is claimed or measured.

Design

Keeping the adaptation at the Java-to-native serialization boundary makes the compatibility rule explicit and leaves Java table metadata unchanged. Using the existing native placeholder preserves the spec/value contract and avoids special cases in task validation. Keeping unknown distinct from identity and void is necessary: neither constant substitution nor forced-null semantics would represent an unrecognized transform correctly. The focused Scala and Rust tests cover both sides of this boundary.

Abstraction & complexity

The helper has a narrow role within the existing transform utilities. Its fixed-name set and parameterized-name matcher reflect the pinned native parser without introducing another registry or reflection layer. Java's supported transform objects emit canonical names, so retaining those names preserves their existing behavior. Future native transform support will need the whitelist and round-trip test updated together. I found no actionable simplification or unnecessary abstraction.

@andygrove
andygrove merged commit 5af898b into apache:mainSep 8, 2026
82 checks passed
@andygrove
andygrove deleted the shady-arbor-0ddb89bf branch September 8, 2026 14:27
@andygrove

Copy link
Copy Markdown
MemberAuthor

Merged. Thanks @sunchao

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Iceberg native scan fails on a table partitioned by an unknown transform (TestForwardCompatibility CI failure)

3 participants

@andygrove@sunchao@rich7420