Skip to content

fix(transform): apply truncate to binary values - #3120

Open
jaideeppyne wants to merge 1 commit into
apache:mainfrom
jaideeppyne:fix-truncate-binary
Open

fix(transform): apply truncate to binary values#3120
jaideeppyne wants to merge 1 commit into
apache:mainfrom
jaideeppyne:fix-truncate-binary

Conversation

@jaideeppyne

Copy link
Copy Markdown

Which issue does this PR close?

What changes are included in this PR?

The spec lists binary under the source types for truncate[W], and the Truncate Transform Details table gives v.subarray(0, L). Transform::result_type already accepts PrimitiveType::Binary, so a binary truncate partition field is legal, but neither half of TransformFunction for Truncate could actually handle one:

  • transform_literal had no PrimitiveLiteral::Binary arm, so Transform::project and strict_project returned FeatureUnsupported for every operator. InclusiveProjection in scan/cache.rs propagates that, so a scan with a filter on such a column errors instead of pruning.
  • transform handled DataType::Binary but not DataType::LargeBinary, and schema_to_arrow_schema maps Iceberg binary to LargeBinary. So the array path failed on the exact arrow type this crate produces for a binary column.

truncate_binary already existed and was correct, it just was not reachable from either entry point. I added the two arms. fixed stays rejected, since the spec does not list it as a truncate source type, and the literal arm is guarded on the datum type so a Fixed datum (also backed by PrimitiveLiteral::Binary) still errors.

Are these changes tested?

Yes, unit tests in truncate.rs: array truncation for BinaryArray and LargeBinaryArray, literal truncation (longer than width, shorter than width, empty), inclusive projection for </<=/>/>=/=/STARTS WITH/IN, strict projection for !=, and that fixed is still rejected. Reverting just the two source arms and keeping the tests fails the 4 new tests and leaves the 16 pre-existing truncate tests passing. Full cargo test -p iceberg is green (1617 tests), fmt and clippy clean.

I found this with a differential harness over bucket[N], truncate[W], year/month/day/hour on a 781-value corpus (spec test vectors plus randoms and boundary values across every source type), running the same inputs through pyiceberg 0.11.1, iceberg-go and this crate's literal and array paths. Of 9333 comparisons, 49 were binary truncate literals where pyiceberg and iceberg-go return a value and this crate errored. The LargeBinary half is not visible that way, I hit it after checking what schema_to_arrow_schema actually emits for binary.

Not covered: BinaryView, which no path in this crate currently produces, and which bucket does not handle either.

AI Disclosure

Claude Code wrote the differential harness and drafted the patch and tests. I checked the spec clauses, verified every claim above against real test output, and reviewed the final diff.

The spec lists binary as a valid source type for truncate[W], and
Transform::result_type accepts it, but TransformFunction for Truncate
rejects it in both paths: transform_literal has no Binary arm, and
transform handles DataType::Binary but not DataType::LargeBinary, which
is what schema_to_arrow_schema produces for an Iceberg binary column.

Closes apache#3119
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

truncate transform rejects binary values in transform_literal and LargeBinary arrays

1 participant