`concat_elements_utf8view` panics with large buffer on 64bit machines
- Dominant language
- Rust
- Stars
- 9.3k
- Forks
- 2.4k
- Avg merge
- 3d 7h
- Merged PRs (30d)
- 344
Description
### Describe the bug
It is possible to cause a panic in Datafusion on 64-bit machines. Datafusion does not handle the panic caused by the underlying `append_value` method in the `GenericByteViewBuilder`. (See [affected line](https://github.com/apache/arrow-rs/blob/b9c2bf73e792e7cb849f0bd453059ceef45b0b74/arrow-array/src/builder/generic_bytes_view_builder.rs#L310))
See the To Reproduce section for the recursive concat query.
Few notes/thoughts:
- The panic is reproducible in the latest version of Data Fusion, but it was also there in the previous versions.
- The panic originates in `arrow-rs` as on 64-bit systems, `usize` can be up to `u64::MAX`, but everything is assumed to be `u32::MAX`. The panic is already declared in the doc of the `append_value` method, but never handled by the consumer (DataFusion). Is there a reason for that?
- I understand the panic happens in `arrow-rs`, but should datafusion handle the panic coming from Arrow? (e.g. in the `append_value` call of `concat_elements_utf8view`) to prevent the panic from happening
- Should datafusion limit somehow the `append_value` calls to prevent the panic from happening?
cc @comphead
### To Reproduce
Sample repository: https://github.com/samueleresca/datafusion-byte-view-builder-issue
1. Run on a 64-bit machine.
2. Include some dummy data (I attached an example):
```
ctx.register_parquet("users","./data/users_shorten.parquet", ParquetReadOptions::default()).await?;
```
3. Run a recursive string concatenation query (see query in `main.rs`)
4. Observe the panic
### Expected behavior
- Should Data Fusion handle the panic?
- (maybe) restrictions on the builder view calls from datafusion
- (maybe) arrow-rs handling this more gracefully
### Additional context
Truncated panic trace happening on my local machine:
```
thread 'tokio-runtime-worker' (38043518) panicked at /Users/samuele/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/arrow-array-56.2.0/src/builder/generic_bytes_view_builder.rs:310:46:
called `Result::unwrap()` on an `Err` value: TryFromIntError(())
stack backtrace:
0: __rustc::rust_begin_unwind
at /rustc/a454fccb02df9d361f1201b747c01257f58a8b37/library/std/src/panicking.rs:698:5
1: core::panicking::panic_fmt
at /rustc/a454fccb02df9d361f1201b747c01257f58a8b37/library/core/src/panicking.rs:75:14
2: core::result::unwrap_failed
at /rustc/a454fccb02df9d361f1201b747c01257f58a8b37/library/core/src/result.rs:1855:5
3: core::result::Result::unwrap
at /Users/samuele/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/result.rs:1226:23
4: arrow_array::builder::generic_bytes_view_builder::GenericByteViewBuilder::append_value
at /Users/samuele/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/arrow-array-56.2.0/src/builder/generic_bytes_view_builder.rs:310:46
5: datafusion_physical_expr::expressions::binary::kernels::concat_elements_utf8view
at /Users/samuele/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/datafusion-physical-expr-50.0.0/src/expressions/binary/kernels.rs:159:20
6: datafusion_physical_expr::expressions::binary::concat_elements
at /Users/samuele/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/datafusion-physical-expr-50.0.0/src/expressions/binary.rs:1078:40
7: datafusion_physical_expr::expressions::binary::BinaryExpr::evaluate_with_resolved_args
at /Users/samuele/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/datafusion-physical-expr-50.0.0/src/expressions/binary.rs:848:29
8: ::evaluate
at /Users/samuele/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/datafusion-physical-expr-50.0.0/src/expressions/binary.rs:479:14
```
Contributor guide
Research direction
Start with datafusion-physical-expr/src/expressions/binary/kernels.rs at concat_elements_utf8view and reproduce the recursive concat query from the linked sample repository's main.rs on a 64-bit machine. Compare the call with arrow-array/src/builder/generic_bytes_view_builder.rs around append_value; done means the query no longer panics when the builder cannot represent the resulting buffer, with focused coverage for the reported case.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- data-engineering
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100