andygrove opened a new issue, #6973: URL: https://github.com/apache/datafusion-comet/issues/6973
### Describe the bug Two code paths in `native/core/src/execution/planner.rs` panic instead of returning an `ExecutionError`, which would take down the executor task rather than failing the query cleanly. 1. **`literal_to_array_ref`, Decimal128 arm** ([planner.rs#L5106-L5125](https://github.com/apache/datafusion-comet/blob/7d8d9b1544b734c85a93cca2cc9d5380ad70accd/native/core/src/execution/planner.rs#L5106-L5125)). Each list-literal decimal value is converted with `BigInt::to_i128().ok_or_else(|| GeneralError(...))` and then immediately `.unwrap()`ped, so a value that does not fit in i128 panics with the error that was meant to be returned: ```rust big_integer .to_i128() .ok_or_else(|| GeneralError(format!("Cannot parse {big_integer:?} as i128 for Decimal literal"))) .unwrap() ``` 2. **`create_binary_expr_with_options`, Try/ANSI checked arithmetic** ([planner.rs#L1594-L1606](https://github.com/apache/datafusion-comet/blob/7d8d9b1544b734c85a93cca2cc9d5380ad70accd/native/core/src/execution/planner.rs#L1594-L1606)). When the eval mode is Try or Ansi and the result type is an integer, any operator other than `+ - * /` hits `todo!("ANSI mode for Operator yet to be implemented!")`. It is not clear whether the serde side can ever send such an operator here, but if it can, the result is a panic. ### Steps to reproduce 1. Build a list literal containing a Decimal128 value whose big-endian bytes exceed the i128 range and plan it through `literal_to_array_ref`. 2. For the second path, plan a binary expression with an integer return type, `EvalMode::Ansi` or `EvalMode::Try`, and an operator outside `Plus/Minus/Multiply/Divide`. ### Expected behavior Both paths return an `ExecutionError` (`GeneralError` is fine) so the query fails with a clear message instead of panicking. ### Additional context Suggested fix: collect the decimal values with `.map(...).collect::<Result<Vec<_>, _>>()?`, and replace the `todo!` with an error return. While in `literal_to_array_ref`, the Binary and Utf8 arms are near-identical builder loops and `Some(nulls.clone().into())` is repeated about a dozen times; a small helper would shrink the function, but that is optional. Line references are pinned to commit 7d8d9b1544b7. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
