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 PR implements option 2 from the issue: a check that takes Arrow values
and applies the writer's conversion. It also takes up alamb's suggestion on
#8551 to make the check an array kernel. It does not add option 1, a public
per-value conversion. For decimals stored as `FIXED_LEN_BYTE_ARRAY`, the
conversion depends on the column's length and scale, so it needs the column
descriptor, which is what `ArrowSbbf` takes. A single value can be checked as a
one-element array (for example with `ScalarValue::to_array`). A public
conversion could be added later if it is wanted.
# What changes are included in this PR?
- The `INT32`, `INT64` and `FIXED_LEN_BYTE_ARRAY` conversions in
`write_leaf` move into `visit_i32`, `visit_i64` and
`visit_fixed_len_byte_array`. These pass either the Arrow values as they are,
or a per-value conversion, to a visitor. The writer and `ArrowSbbf` share this
one copy of the conversion. The writer borrows or collects exactly as before,
and `ArrowSbbf` hashes values one at a time without collecting them.
- New `parquet::arrow::bloom_filter::ArrowSbbf`. It is built from an `Sbbf`
and the column's `ColumnDescriptor`. Its `check(&dyn Array) ->
Result<BooleanArray>` applies the writer's conversion for the column's physical
type before checking each value.
- Nulls stay null, and a `Null` array returns all nulls.
- Dictionary and run-end encoded arrays check their values and map the
results through the keys or runs.
- Decimals of any precision can be checked against a decimal column of the
same scale. They are converted to the column's physical type and length, and
truncated in the same way as the writer.
- It returns an error in three cases: for a type the writer could not have
written to that column, for a decimal array of a different scale (or a decimal
against a non-decimal column, or the reverse), and for a `FixedSizeBinary`
array of a different width.
- The `ArrowSbbf` docs have a table of the Arrow types whose physical
representation changes when written.
- `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`, including truncation to days;
- `Decimal128` at precisions 5, 15 and 30 (stored as `INT32`, `INT64` and
`FIXED_LEN_BYTE_ARRAY`);
- decimals checked at a different precision or a narrower type than the
column, values too large for their precision, and a different scale;
- `FixedSizeBinary`, `Float16`, intervals and strings;
- nulls, `Null` arrays, dictionary arrays and run-end encoded arrays.
Other tests cover the errors for mismatched types. 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`, the tests and the changes for the review comments,
from the plan in the issue and the reviews. 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]
