bhaskargurram-ai opened a new pull request, #11549:
URL: https://github.com/apache/arrow-rs/pull/11549
# Which issue does this PR close?
- Closes #5550
# Rationale for this change
`ArrowWriter` converts some Arrow types before writing them: `Int8`,
`Int16`, `UInt8` and `UInt16` become `INT32`, `Date64` becomes `INT32` days
(with `coerce_types`), and decimals become `INT32`, `INT64` or
`FIXED_LEN_BYTE_ARRAY` depending on precision. The bloom filter is built from
the converted values. So `Sbbf::check` with the original Arrow value hashes
different bytes and reports values that are in the file as absent. This is why
DataFusion turned off bloom filter pruning for these types
(apache/datafusion#9779).
This follows the plan in the issue: options 1 and 2, which alamb said
sounded reasonable. It also takes up alamb's suggestion on #8551 to make the
check an array kernel.
# What changes are included in this PR?
- The `INT32`, `INT64` and `FIXED_LEN_BYTE_ARRAY` conversions in
`write_leaf` move into three crate-private helpers: `coerce_to_i32`,
`coerce_to_i64` and `coerce_to_fixed_len_byte_array`. The match arms are
unchanged. The writer and the new check now share one copy of the conversion
and cannot drift apart.
- New `parquet::arrow::bloom_filter::ArrowSbbf`. It is built from an `Sbbf`
and the column's `ColumnDescriptor`, and its `check(&dyn Array) ->
Result<BooleanArray>` applies the writer's conversion for the column's physical
type before checking each value.
- Nulls stay null.
- Dictionary arrays check their values and map them through the keys.
- A type the writer could not have written to that column returns an error.
- `Sbbf::check` docs now point Arrow users to `ArrowSbbf`.
# Are these changes tested?
Yes. New round-trip tests in `parquet/src/arrow/bloom_filter.rs` write each
type with `ArrowWriter` and a bloom filter, read the filter back, and check
that every written value is found and a few absent values are not. The types
covered are:
- `Int8`, `Int16`, `UInt8`, `UInt16`, including min and max values;
- `UInt32` and `UInt64` above the signed max;
- `Date64` with and without `coerce_types`;
- `Decimal128` at precisions 5, 15 and 30 (stored as `INT32`, `INT64` and
`FIXED_LEN_BYTE_ARRAY`);
- `Float16` and strings.
Other tests cover nulls, dictionary arrays, and the error for a mismatched
type. The doc example shows that `Sbbf::check(&1_i8)` misses a value that
`ArrowSbbf` finds. The existing writer tests cover the refactor.
# Are there any significant user-facing changes?
There is a new public API, `parquet::arrow::bloom_filter::ArrowSbbf`. No
existing API changes.
AI usage: I used Claude Code while writing this. It drafted the helper
extraction, `ArrowSbbf` and the tests from the plan in the issue. I reviewed
every line and checked the conversions against `write_leaf`. I can debug and
maintain 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]
