bhaskargurram-ai commented on code in PR #11549:
URL: https://github.com/apache/arrow-rs/pull/11549#discussion_r4275897745
##########
parquet/src/arrow/arrow_writer/mod.rs:
##########
@@ -1812,6 +1608,241 @@ fn write_leaf(
}
}
+/// Returns the values [`ArrowWriter`] stores for `column` in an `INT32` column
+///
+/// These are also the values hashed into the column's bloom filter
+pub(crate) fn coerce_to_i32(column: &dyn arrow_array::Array) ->
Result<ScalarBuffer<i32>> {
Review Comment:
Good call, I reworked this in 8b4abea. The conversion `match` now lives in
`visit_i32`/`visit_i64`, and each arm hands a small visitor either
`slice(&[T])` when the Arrow values are stored as they are, or `map(&[S], |s|
-> T)` when each value is converted. The writer's visitor writes the borrowed
slice directly and, for `map`, collects into a `Vec` and writes that, so it's
the same one allocation the `unary` calls made before and the passthrough arms
stay zero-copy (`Decimal32` to INT32 actually stops copying). `ArrowSbbf`'s
visitor just hashes each value as it goes, so the only allocation on the check
path is the result `BooleanBuffer`.
A plain per-value iterator didn't fit on its own because `write_primitive`
needs a contiguous `&[i32]`/`&[i64]`, and I didn't want the writer to collect
values it can currently borrow. Splitting into `slice` and `map` keeps both. I
did the same for FIXED_LEN_BYTE_ARRAY: the arms now pass a width and a
per-value `fill` closure, the writer still fills one arena, and the check
reuses a single scratch buffer. And agreed on `arrow_cast::cast`: the UInt32
reinterpret, the decimal `as` casts and the Date64 to days conversion don't
match cast semantics, so these have to stay the writer's own conversions.
On one generic function: I kept two. The parts that really were the same
(writing a slice, collecting a mapped slice, hashing per value) are now written
once, generic over the target type. What's left in `visit_i32` and `visit_i64`
is two different tables: Int8/Int16/UInt8/UInt16/UInt32/Date32/Time32/Null only
go to INT32; Int64/UInt64/Time64/Timestamp/Duration only to INT64; Date64
truncates to days for INT32 but is stored as-is for INT64; Decimal32 is stored
as-is in INT32, while Decimal64 is narrowed for INT32 and as-is for INT64. A
generic version would move those two tables into two trait impls without
removing them, and make them harder to check against `arrow_to_parquet_type`.
Happy to change it if you see a shape I'm missing.
--
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]
