yuefdev opened a new pull request, #11087:
URL: https://github.com/apache/arrow-rs/pull/11087

   # Which issue does this PR close?
   
   - Closes #11073.
   
   # Rationale for this change
   
   `ByteArrayEncoder`, which the arrow writer uses for `Binary`/`Utf8` and the 
other byte array types, computes min/max with plain byte order. For a 
`BYTE_ARRAY` column annotated as `DECIMAL` the values are big-endian two's 
complement, so the statistics come out as if the values were unsigned: writing 
-1, 0, 1 gives min `0x00` and max `0xFF`.
   
   The non-arrow column writer and the `FixedSizeBinary` path already use 
`compare_greater_byte_array_decimals` for decimals, which is why only this path 
was affected.
   
   # What changes are included in this PR?
   
   - `ByteArrayEncoder` records whether the column is a decimal (logical or 
converted type) and, if so, uses `compare_greater_byte_array_decimals` when 
computing and merging min/max. Other columns still use the plain byte 
comparison.
   - `compute_min_max` takes the comparison as a generic parameter, so there is 
no per-value branch, and `compare_greater_byte_array_decimals` is now 
`pub(crate)`.
   
   # Are these changes tested?
   
   Yes. I added `test_byte_array_decimal_statistics`, which writes -1, 0, 1 
from a `BinaryArray` into a `BYTE_ARRAY` `DECIMAL(2, 0)` column. It fails on 
`main` (min is `[0]` rather than `[255]`) and passes with this change. I also 
ran the reproducer from the issue against this branch: all three paths now 
report `(-1, 1)`.
   
   Local runs (Windows, Rust 1.98.1):
   
   - `cargo test -p parquet`: all passed (1382 lib, 96 `arrow_reader`, 15 
`arrow_writer`, 73 doc tests, plus the smaller suites)
   - `cargo test -p parquet --all-features --no-fail-fast`: lib (1436) and all 
integration suites passed. The only test failure, `parquet-fromcsv` 
`test_command_help`, also fails on `main` here because `core.autocrlf` checks 
the help text out with CRLF. The all-features doc tests didn't build on this 
machine (stale crate artefacts in `target/` and a pagefile limit), so I'm 
relying on CI for those
   - `cargo clippy -p parquet --all-targets --all-features -- -D warnings`: 
clean
   - `cargo fmt --all -- --check`: clean
   
   # Are there any user-facing changes?
   
   Statistics for `BYTE_ARRAY` decimal columns written through the arrow writer 
are now correct. No API changes.
   
   AI disclosure: the investigation, fix, test and this description were 
produced with Claude Code (Claude Opus 5). The test results above come from 
real local runs.
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
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]

Reply via email to