viirya opened a new pull request, #2710: URL: https://github.com/apache/iceberg-rust/pull/2710
## Which issue does this PR close? - Closes #2698. ## What changes are included in this PR? `type_to_arrow_type` mapped `PrimitiveType::Binary` to `DataType::LargeBinary`. This is inconsistent with: 1. **Java** — `ArrowSchemaUtil` maps Iceberg `BinaryType` to Arrow `Binary` (`ArrowType.Binary.INSTANCE`), and `FixedType` to `FixedSizeBinary`. 2. **parquet-rs** — an unannotated Parquet `BYTE_ARRAY` column (no UTF8/String logical type) is read as `DataType::Binary` by default; `LargeBinary` only appears when the file's embedded Arrow schema metadata or a user-supplied schema forces it. The mismatch forces an unnecessary cast when reading Binary columns and diverges from the rest of the ecosystem. This PR maps `PrimitiveType::Binary` to `DataType::Binary` (`crates/iceberg/src/arrow/schema.rs`). The conversions that go the other way already accept both representations, so externally produced `LargeBinary` columns keep working: - `arrow_schema_to_schema` accepts `Binary | LargeBinary | BinaryView` → Iceberg `Binary`. - the arrow-to-literal path (`arrow/value.rs`) downcasts to `LargeBinaryArray` *or* `BinaryArray`. - the bucket transform handles both `DataType::Binary` and `DataType::LargeBinary`. `Fixed` is unaffected (still `FixedSizeBinary`, with the rarely-hit `LargeBinary` fallback for lengths exceeding `i32::MAX` left in place). Test fixtures that asserted the old output were updated: the `schema_to_arrow_schema` round-trip fixture (the two `binary` fields and the `list<binary>` element) and the writer tests that fed `LargeBinaryArray` for an Iceberg `Binary` column (`parquet_writer`, `equality_delete_writer`) now use `BinaryArray`. ## Are these changes tested? Existing coverage. `test_schema_to_arrow_schema` / `test_arrow_schema_to_schema` exercise both directions of the mapping and now assert `Binary`; the two writer tests round-trip an Iceberg `Binary` column end-to-end through Parquet. Full `iceberg` lib suite (1312 tests) passes, including all `arrow::` and `writer::` tests; clippy and rustfmt clean. Note on overlap with #2668: that PR added a `LargeBinary` arm to `create_primitive_array_repeated` for `_partition` columns under the old mapping. With this change that arm is no longer exercised for Iceberg Binary, but it's harmless to keep as a fallback — whichever lands second can drop it. -- 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]
