bhaskargurram-ai opened a new pull request, #11741:
URL: https://github.com/apache/arrow-rs/pull/11741
# Which issue does this PR close?
- Part of #5550.
This is the write-path refactor from #11549, split out as the reviewers
asked so it can be reviewed and benchmarked on its own. #11549 will then only
add `ArrowSbbf` on top of it.
# Rationale for this change
`ArrowSbbf` in #11549 checks Arrow values against a bloom filter written by
`ArrowWriter`, so it has to apply the same conversions the writer applies
before hashing (`Int8` to `INT32`, `Date64` to days, decimals to `INT32`,
`INT64` or `FIXED_LEN_BYTE_ARRAY`, and so on). Instead of a second copy of
those conversions that could drift from the writer, I moved them out of
`write_leaf` into functions both can use.
# What changes are included in this PR?
- The `INT32`, `INT64` and `FIXED_LEN_BYTE_ARRAY` arms of `write_leaf` move
into crate-private `visit_i32`, `visit_i64` and `visit_fixed_len_byte_array`.
These pass the values to a visitor:
- `PrimitiveVisitor` gets either the Arrow values as they are (`slice`) or
the values with a per-value conversion (`map`).
- `FixedLenByteArrayVisitor` gets the value length and a closure that
fills each value.
- The writer implements these with `WritePrimitive` and
`WriteFixedLenByteArray`. Types that were written straight from the Arrow
buffer still are. Types that went through `unary` are collected into a `Vec`
with the same conversion. FLBA values are still built with `chunk_array_slice`.
The one difference is `Decimal32` to `INT32`, which now borrows the buffer
instead of copying it through an identity `unary`.
- The `get_*_array_slice` helpers for intervals, decimals, `Float16` and
`FixedSizeBinary` are folded into `visit_fixed_len_byte_array` and
`visit_decimal`. The match arms, the error messages and the decimal length
`assert!` are unchanged.
The parts only `ArrowSbbf` needs (its own error for unsupported types, a
`Decimal32` arm in `visit_i64`, and sign extension for decimal columns wider
than the Arrow type) stay in #11549.
# Are these changes tested?
There is no change in behaviour, so I didn't add tests. The existing
`arrow_writer` tests cover every arm.
adriangb benchmarked these write paths on #11549 and they were within noise
of main. Could someone run `run benchmark arrow_writer` on this PR as well?
# Are there any significant user-facing changes?
No. The new functions and traits are crate-private.
--
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]
