viirya commented on code in PR #2710:
URL: https://github.com/apache/iceberg-rust/pull/2710#discussion_r4459566762
##########
crates/iceberg/src/arrow/schema.rs:
##########
@@ -890,7 +890,8 @@ impl SchemaVisitor for ToArrowSchemaConverter {
.map(DataType::FixedSizeBinary)
.unwrap_or(DataType::LargeBinary),
)),
- PrimitiveType::Binary | PrimitiveType::Geometry(_) |
PrimitiveType::Geography(_) => {
+ PrimitiveType::Binary =>
Ok(ArrowSchemaOrFieldOrType::Type(DataType::Binary)),
Review Comment:
You're right, and my earlier reply was wrong about this path: the input
arrays are unchanged, but `calculate()` builds the struct against
`partition_arrow_type`, which is now `Binary`. Reproduced with a test, then
fixed in be68461ee by moving the `LargeBinary` → `Binary` cast from
`project_batch()` into `project_column()`, so both the equality delete writer
and `PartitionValueCalculator` get columns matching the projected schema.
The regression test
`test_partition_calculator_binary_partition_accepts_large_binary_input` covers
identity and truncate partitions on a binary field. Truncate is included
because it only handled `BinaryArray`, so it rejected `LargeBinary` input even
before this PR; the cast fixes that too.
I also rechecked the read side: `record_batch_transformer` builds targets
from `type_to_arrow_type`, but its `PromotePlan` falls back to `cast` on a type
mismatch, so `LargeBinary` columns from files written by older versions are
cast to `Binary`.
--
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]
