Repository navigation
Add ArrowSbbf to check Arrow arrays against Parquet bloom filters - #11549
Open
bhaskargurram-ai wants to merge 7 commits into
Open
bhaskargurram-ai wants to merge 7 commits into
bhaskargurram-ai wants to merge 7 commits into
Conversation
…rowWriter ArrowWriter converts some Arrow types before writing them (Int8, Int16, UInt8 and UInt16 to INT32, Date64 to INT32 days, decimals to INT32, INT64 or FIXED_LEN_BYTE_ARRAY, ...) and builds the bloom filter from the converted values, so Sbbf::check with the original Arrow value reports values that are present as absent. Move the writer's INT32, INT64 and FIXED_LEN_BYTE_ARRAY conversions into shared helpers and add parquet::arrow::bloom_filter::ArrowSbbf, whose check(&dyn Array) applies the same conversion and returns a BooleanArray. Closes apache#5550
mbutrovich
reviewed
Oct 14, 2026
mbutrovich
left a comment
mbutrovich
left a comment
Contributor
There was a problem hiding this comment.
Thanks @bhaskargurram-ai, took a first pass.
…ArrowSbbf ArrowSbbf::check converted decimals for a FIXED_LEN_BYTE_ARRAY column to the byte width of the checked array's precision, but the writer uses the column's width, so a value written at precision 30 (13 bytes) was not found when checked with a precision 20 or 38 array. Convert decimals to the column's type_length instead: sign-extend narrower values and drop leading sign bytes from wider ones, reporting a value that does not fit as absent. Decimal values with a different scale than the column's are different unscaled integers, so checking them now returns an error, as does checking a FixedSizeBinary array whose length differs from the column's. Null arrays now return all nulls for any column type. Replace coerce_to_i32, coerce_to_i64 and coerce_to_fixed_len_byte_array, which collected the converted values into buffers, with visit_i32, visit_i64 and visit_fixed_len_byte_array. They keep the single copy of the conversion from each Arrow type and pass either the Arrow values as they are or a per-value conversion to a visitor. The writer's visitors borrow the Arrow buffer when no conversion is needed and collect converted values as before, while ArrowSbbf hashes each value without collecting them. Decimal32 values written to an INT32 column are no longer copied.
adriangb
reviewed
Oct 18, 2026
adriangb
left a comment
adriangb
left a comment
Contributor
There was a problem hiding this comment.
Thanks, this is a good fix!
Some requests:
- We need a
arrow_writerbenchmark run (before and after). This PR changes the writer hot path forINT32,INT64andFIXED_LEN_BYTE_ARRAY. Ill kick it off. - Option 1 in #5550 asks for a per-value conversion that callers can use before
Sbbf::check. Thevisit_*functions arepub(crate), so this PR gives only option 2. Please update the PR description, or make a per-value conversion public. - #5550 also asks to document which Arrow types change their physical representation. A short table in the
ArrowSbbfdocs would be sufficient.
I ran randomized round-trip tests over all writer types (about 15k configurations and 114k cross-precision decimal checks) and cargo-mutants on the changed code. I found no false negatives for in-range values and no panics. The writer output is byte-identical to the base commit. The inline comments show the gaps that I found.
| let indices = self.levels.non_null_indices().iter().copied(); | ||
| // A value that does not fit is truncated, as the writer has always done | ||
| let values = chunk_array_slice(length, indices, |i, value| { | ||
| fill(i, value); |
Contributor
There was a problem hiding this comment.
The writer ignores the bool, but it still pays for the overflow check on each value (dropped.iter().all(..)). Before this PR the width check was done once, outside the loop. Please confirm with a benchmark that this has no cost for Decimal128 to FIXED_LEN_BYTE_ARRAY.
Also, the old assert!(chunk_size <= T::BYTE_LENGTH) is gone. Was that intentional?
Comment on lines
+1912
to
+1916
| out.copy_from_slice(kept); | ||
| let sign = if kept.first().is_some_and(|b| (*b as i8) < 0) { | ||
| 0xFF | ||
| } else { | ||
| 0 |
Contributor
There was a problem hiding this comment.
This sign-byte expression is also used above. A small helper removes the duplicate:
Suggested change
| out.copy_from_slice(kept); | |
| let sign = if kept.first().is_some_and(|b| (*b as i8) < 0) { | |
| 0xFF | |
| } else { | |
| 0 | |
| let sign = sign_byte(kept); |
fn sign_byte(bytes: &[u8]) -> u8 {
if bytes.first().is_some_and(|b| (*b as i8) < 0) { 0xFF } else { 0 }
}| sbbf: &'a Sbbf, | ||
| physical_type: PhysicalType, | ||
| /// The length of the column's values if it is a `FIXED_LEN_BYTE_ARRAY` column | ||
| type_length: i32, |
Contributor
There was a problem hiding this comment.
This is cast as usize later. Do the conversion once in new and store a usize:
Suggested change
| type_length: i32, | |
| type_length: usize, |
| // Never negative for a FIXED_LEN_BYTE_ARRAY column, which the schema checks | ||
| type_length: self.type_length as usize, | ||
| }; | ||
| visit_fixed_len_byte_array(array, visitor)?? |
Contributor
There was a problem hiding this comment.
Nit: the double ?? is hard to read. Maybe make the visitor Output the inner type, or add a short comment that explains the two layers of Result.
| DataType::Decimal32(_, scale) | ||
| | DataType::Decimal64(_, scale) | ||
| | DataType::Decimal128(_, scale) | ||
| | DataType::Decimal256(_, scale) => self.validate_decimal_scale(*scale)?, |
Contributor
There was a problem hiding this comment.
The check rejects decimal values for a non-decimal column. But the reverse is not rejected: an Int8 array checked against an INT32 decimal or Date32 column is hashed with no error. This cannot give a false negative, but the behavior is not symmetric. Should a decimal column also require a decimal array?
| }, | ||
| ArrowDataType::Decimal64(_, _) => { | ||
| visitor.slice(column.as_primitive::<Decimal64Type>().values()) | ||
| } |
Contributor
There was a problem hiding this comment.
visit_i64 has no Decimal32 arm. Thus check with a Decimal32(9, 2) array against a Decimal64(12, 2) column returns Cannot coerce Decimal32(9, 2) to I64. The ArrowSbbf docs say that decimals of any precision can be checked against a column of the same scale.
Suggested change
| } | |
| } | |
| ArrowDataType::Decimal32(_, _) => { | |
| visitor.map(column.as_primitive::<Decimal32Type>().values(), |v| v as i64) | |
| } |
| ArrowDataType::Date64 => { | ||
| // If the column is a Date64, we truncate it | ||
| visitor.map(column.as_primitive::<Date64Type>().values(), |x| { | ||
| (x / 86_400_000) as i32 |
Contributor
There was a problem hiding this comment.
A mutation test that changes / to * here is not detected by the tests. Please add a test that writes Date64 5 * 86_400_000 and -1, and checks that the bloom filter contains 5i32 and 0i32.
| } | ||
|
|
||
| match array.data_type() { | ||
| DataType::Null => return Ok(BooleanArray::new_null(array.len())), |
Contributor
There was a problem hiding this comment.
The tests pass if this arm is removed, because they check Null only against an INT32 column. Please add a test that checks NullArray::new(2) against a Float64 or Utf8 column and expects [None, None].
| impl FixedLenByteArrayVisitor for CheckFixedLenByteArray<'_> { | ||
| type Output = Result<BooleanBuffer>; | ||
|
|
||
| fn decimal_length(&self, _precision: u8) -> usize { |
Contributor
There was a problem hiding this comment.
Arrow does not validate decimal precision, so Decimal128(20, 2) can hold i128::MAX / 3. For a FIXED_LEN_BYTE_ARRAY column the writer truncates this value, but check on the same array returns false. For INT32 and INT64 columns, check truncates like the writer and returns true. Please make the two paths the same, or document the difference.
| | DataType::Decimal64(_, scale) | ||
| | DataType::Decimal128(_, scale) | ||
| | DataType::Decimal256(_, scale) => self.validate_decimal_scale(*scale)?, | ||
| _ => {} |
Contributor
There was a problem hiding this comment.
Two small points:
- The writer accepts
RunEndEncodedcolumns, butcheckreturnsCannot coerce RunEndEncoded(..) to I32. Please support REE (check the values, then map by run ends), or document this. - A type mismatch against a
FIXED_LEN_BYTE_ARRAYcolumn returnsNYI: Attempting to write an Arrow type that is not yet implemented. That message is incorrect for a check.
Contributor
|
run benchmark arrow_writer |
|
🤖 Benchmark starting (GKE) | trigger Target: arrow_writer Sharding factor: 1 (1 workers). Comparing fix/bloom-filter-arrow-check (8b4abea) to bece8ca (merge-base) diff Run configurationrun benchmark arrow_writer
shards: 1
Results will be posted when all workers finish. File an issue against this benchmark runner |
Contributor
|
run benchmark arrow_writer shards: 4 |
|
🤖 Benchmark starting (GKE) | trigger Target: arrow_writer Sharding factor: 4 (4 workers). Comparing fix/bloom-filter-arrow-check (8b4abea) to bece8ca (merge-base) diff Run configurationrun benchmark arrow_writer
shards: 4
Results will be posted when all workers finish. File an issue against this benchmark runner |
…in ArrowSbbf - Choose the decimal resize once per array instead of checking each value, so the writer's FIXED_LEN_BYTE_ARRAY loop is the same as before, and restore the assert that the precision fits the native decimal type. - Truncate decimals that are too large for a FIXED_LEN_BYTE_ARRAY column in ArrowSbbf the same way the writer does, as the INT32 and INT64 paths already do, so a written value is never reported as absent. - Check run-end encoded arrays by their values, and return a check-specific error for an Arrow type that cannot be stored in the column. - Require a decimal array for a decimal column, as well as the reverse. - Check Decimal32 values against INT64 decimal columns. - Visitor methods return Result, so the visit functions return a single Result. Store the column length as usize. Add a sign_byte helper. - Document which Arrow types change their physical representation. - Test Date64 to days, Null arrays against DOUBLE and BYTE_ARRAY columns, Decimal32 against an INT64 column, out-of-precision decimals, REE arrays and the new errors.
|
🤖 Benchmark completed (GKE) | trigger Target: arrow_writer Comparing fix/bloom-filter-arrow-check (8b4abea) to bece8ca (merge-base) diff Run configurationrun benchmark arrow_writer
shards: 4
Comparison results: View the complete native critcmp output. The comparison is published in an unlisted Gist because the original inline report exceeded GitHub's comment limit. Per-runner informationarrow_writer — shard 1/4Node: gk3-benchmark-cluster-nap-1iq4pjyb-5c94fccf-v9sf Instance: c4a-highmem-16 (12 vCPU / 65 GiB) uname: BENCH_COMMAND: cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writerCPU Details (lscpu)Resource Usagebase (merge-base)
branch
arrow_writer — shard 2/4Node: gk3-benchmark-cluster-nap-1iq4pjyb-5c94fccf-l97c Instance: c4a-highmem-16 (12 vCPU / 65 GiB) uname: BENCH_COMMAND: cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writerCPU Details (lscpu)Resource Usagebase (merge-base)
branch
arrow_writer — shard 3/4Node: gk3-benchmark-cluster-nap-1rvnyzuz-5973f194-q2kq Instance: c4a-highmem-16 (12 vCPU / 65 GiB) uname: BENCH_COMMAND: cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writerCPU Details (lscpu)Resource Usagebase (merge-base)
branch
arrow_writer — shard 4/4Node: gk3-benchmark-cluster-nap-1rvnyzuz-7240f749-29lj Instance: c4a-highmem-16 (12 vCPU / 65 GiB) uname: BENCH_COMMAND: cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writerCPU Details (lscpu)Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
|
🤖 Benchmark failed or incomplete (GKE) | trigger Target: arrow_writer Comparing fix/bloom-filter-arrow-check (8b4abea) to bece8ca (merge-base) diff Run configurationrun benchmark arrow_writer
shards: 1
Errors / missing shardsPer-runner informationarrow_writer — shard 1/1Node: gk3-benchmark-cluster-nap-1rvnyzuz-894d94a5-2x5v Instance: c4a-highmem-16 (12 vCPU / 65 GiB) uname: BENCH_COMMAND: cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writerCPU Details (lscpu)Resource UsageNo completed measurement resource samples. File an issue against this benchmark runner |
Contributor
|
run benchmark arrow_writer shards: 8 |
|
🤖 Benchmark starting (GKE) | trigger Target: arrow_writer Sharding factor: 8 (8 workers). Comparing fix/bloom-filter-arrow-check (77a7137) to bece8ca (merge-base) diff Run configurationrun benchmark arrow_writer
shards: 8
Results will be posted when all workers finish. File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Target: arrow_writer Comparing fix/bloom-filter-arrow-check (77a7137) to bece8ca (merge-base) diff Run configurationrun benchmark arrow_writer
shards: 8
Comparison results: View the complete native critcmp output. The comparison is published in an unlisted Gist because the original inline report exceeded GitHub's comment limit. Per-runner informationarrow_writer — shard 1/8Node: gk3-benchmark-cluster-nap-28gt8wfj-9d94965e-tzhv Instance: c4a-highmem-16 (12 vCPU / 65 GiB) uname: BENCH_COMMAND: cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writerCPU Details (lscpu)Resource Usagebase (merge-base)
branch
arrow_writer — shard 2/8Node: gk3-benchmark-cluster-nap-28gt8wfj-cf05c2d9-hm2z Instance: c4a-highmem-16 (12 vCPU / 65 GiB) uname: BENCH_COMMAND: cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writerCPU Details (lscpu)Resource Usagebase (merge-base)
branch
arrow_writer — shard 3/8Node: gk3-benchmark-cluster-nap-i0c7et67-c69b8f21-hb79 Instance: c4a-highmem-16 (12 vCPU / 65 GiB) uname: BENCH_COMMAND: cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writerCPU Details (lscpu)Resource Usagebase (merge-base)
branch
arrow_writer — shard 4/8Node: gk3-benchmark-cluster-nap-28gt8wfj-c1668e86-vvb7 Instance: c4a-highmem-16 (12 vCPU / 65 GiB) uname: BENCH_COMMAND: cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writerCPU Details (lscpu)Resource Usagebase (merge-base)
branch
arrow_writer — shard 5/8Node: gk3-benchmark-cluster-nap-28gt8wfj-9d94965e-bwws Instance: c4a-highmem-16 (12 vCPU / 65 GiB) uname: BENCH_COMMAND: cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writerCPU Details (lscpu)Resource Usagebase (merge-base)
branch
arrow_writer — shard 6/8Node: gk3-benchmark-cluster-nap-28gt8wfj-c1668e86-vvb7 Instance: c4a-highmem-16 (12 vCPU / 65 GiB) uname: BENCH_COMMAND: cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writerCPU Details (lscpu)Resource Usagebase (merge-base)
branch
arrow_writer — shard 7/8Node: gk3-benchmark-cluster-nap-i0c7et67-c69b8f21-hb79 Instance: c4a-highmem-16 (12 vCPU / 65 GiB) uname: BENCH_COMMAND: cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writerCPU Details (lscpu)Resource Usagebase (merge-base)
branch
arrow_writer — shard 8/8Node: gk3-benchmark-cluster-nap-28gt8wfj-9d94965e-bwws Instance: c4a-highmem-16 (12 vCPU / 65 GiB) uname: BENCH_COMMAND: cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writerCPU Details (lscpu)Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
The arrow_writer benchmark on aarch64 still showed the writer slower than the base commit with the shared visitors, including on paths whose per-value work did not change. Restore arrow_writer/mod.rs to the base commit, so this PR no longer changes the writer, and give ArrowSbbf its own copy of the INT32, INT64 and FIXED_LEN_BYTE_ARRAY conversions. With the writer's source restored, adding ArrowSbbf to the crate still changed the writer's machine code, because it changes how the crate is split into codegen units and inlined. Mark ArrowSbbf's non-generic functions #[inline] so they are only compiled in crates that call them. The arrow_writer benchmark binary is then identical to the base commit's. ArrowSbbf behaves as before: FIXED_LEN_BYTE_ARRAY decimals are truncated to the column's length like the writer, narrower decimals are sign-extended, and run-end encoded and dictionary arrays are checked by their values. To keep the two copies from drifting apart, add a test that writes a column of every type ArrowWriter supports, with and without coerce_types, and checks that ArrowSbbf finds every written value in the column's bloom filter. Also test a decimal column longer than 32 bytes.
Contributor
|
@bhaskargurram-ai I ran the
|
Contributor
Author
@adriangb Thanks for running those, good to know it was noise. I’ll put the shared write paths back in this PR (keeping the every-type test as a guard against drift) and ping you for a benchmark run once it’s pushed. |
Contributor
|
are you opposed to splitting out the refactor pr as a precursor? i think both @mbutrovich and i would prefer that |
Benchmarks on a dedicated machine showed no writer slowdown from the shared visitors, so go back to them: ArrowWriter and ArrowSbbf again use the same INT32, INT64 and FIXED_LEN_BYTE_ARRAY conversions through visit_i32, visit_i64 and visit_fixed_len_byte_array, and ArrowSbbf's own copy is removed. The #[inline] attributes are removed with it, since they were only there to keep the writer's machine code unchanged. Keep the test that writes a column of every type ArrowWriter supports and checks that ArrowSbbf finds every written value, and the test for a decimal column longer than 32 bytes.
Contributor
Author
@adriangb Not opposed at all, happy to split it. I’ll open the write-path refactor (the shared visit_i32 / visit_i64 / visit_fixed_len_byte_array) as its own PR so it can be reviewed and benchmarked by itself, and this PR will then only add ArrowSbbf on top of it. |
Contributor
Author
|
Opened #11741 with just the write-path refactor (visit_i32 / visit_i64 / visit_fixed_len_byte_array), so it can be reviewed and benchmarked on its own. Once that lands I’ll merge main into this branch, and this PR will only add ArrowSbbf (plus the small hooks it needs in those visitors). |
Contributor
|
Marking this as draft to unblock #11741 |
etseidl
marked this pull request as draft
October 28, 2026 22:17
etseidl
marked this pull request as ready for review
October 28, 2026 22:45
Contributor
|
I tried rebasing on main, but don't have push permission for this PR. @bhaskargurram-ai can you please rebase on main? |
jaideeppyne
pushed a commit
to jaideeppyne/arrow-rs
that referenced
this pull request
Oct 28, 2026
… visitors (apache#11741) # Which issue does this PR close? - Part of apache#5550. This is the write-path refactor from apache#11549, split out as the reviewers asked so it can be reviewed and benchmarked on its own. apache#11549 will then only add `ArrowSbbf` on top of it. # Rationale for this change `ArrowSbbf` in apache#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 apache#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 apache#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. --------- Co-authored-by: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Co-authored-by: Ed Seidl <etseidl@users.noreply.github.com>
…row-check # Conflicts: # parquet/src/arrow/arrow_writer/mod.rs
Contributor
Author
|
Thanks @etseidl. I merged main into the branch (947b3e9) rather than rebasing, so the history stays as it was. Now that #11741 is in, this PR is just ArrowSbbf on top of the shared visitors. The writer only gains the small hooks ArrowSbbf needs: an unsupported hook on the visitor traits, Decimal32 in visit_i64, and sign extension in visit_decimal. Parquet tests and clippy pass locally. It's ready for review again. |
etseidl
reviewed
Oct 29, 2026
etseidl
left a comment
etseidl
left a comment
Contributor
There was a problem hiding this comment.
I'll do a better pass tomorrow, but here's a nit my agent found.
A timestamp, time or duration is checked in the unit of the probe array's data type, so it must match the unit the column was written in. Add a test showing that the same instant in another unit is not found.
etseidl
approved these changes
Oct 29, 2026
etseidl
left a comment
etseidl
left a comment
Contributor
There was a problem hiding this comment.
Thanks @bhaskargurram-ai, no more nits from me.
@adriangb are all of your concerns addressed?
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Rationale for this change
ArrowWriterconverts some Arrow types before writing them:Int8,Int16,UInt8andUInt16becomeINT32,Date64becomesINT32days (withcoerce_types), and decimals becomeINT32,INT64orFIXED_LEN_BYTE_ARRAYdepending on precision. The bloom filter is built from the converted values. SoSbbf::checkwith 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 whatArrowSbbftakes. A single value can be checked as a one-element array (for example withScalarValue::to_array). A public conversion could be added later if it is wanted.What changes are included in this PR?
INT32,INT64andFIXED_LEN_BYTE_ARRAYconversions inwrite_leafmove intovisit_i32,visit_i64andvisit_fixed_len_byte_array. These pass either the Arrow values as they are, or a per-value conversion, to a visitor. The writer andArrowSbbfshare this one copy of the conversion. The writer borrows or collects exactly as before, andArrowSbbfhashes values one at a time without collecting them.parquet::arrow::bloom_filter::ArrowSbbf. It is built from anSbbfand the column'sColumnDescriptor. Itscheck(&dyn Array) -> Result<BooleanArray>applies the writer's conversion for the column's physical type before checking each value.Nullarray returns all nulls.FixedSizeBinaryarray of a different width.ArrowSbbfdocs have a table of the Arrow types whose physical representation changes when written.Sbbf::checkdocs now point Arrow users toArrowSbbf.Are these changes tested?
Yes. New round-trip tests in
parquet/src/arrow/bloom_filter.rswrite each type withArrowWriterand 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;UInt32andUInt64above the signed max;Date64with and withoutcoerce_types, including truncation to days;Decimal128at precisions 5, 15 and 30 (stored asINT32,INT64andFIXED_LEN_BYTE_ARRAY);FixedSizeBinary,Float16, intervals and strings;Nullarrays, 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 thatArrowSbbffinds. 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 againstwrite_leaf. I can debug and maintain it.