jonasHanhan opened a new pull request, #27072:
URL: https://github.com/apache/datafusion/pull/27072

   ## Which issue does this PR close?
   
   - Closes #27059.
   
   ## Rationale for this change
   
   Substrait's `functions_arithmetic.yaml` defines `shift_left` and 
`shift_right`, but the consumer only recognised `bitwise_shift_left` / 
`bitwise_shift_right`. A plan from another producer that uses the standard 
names fails with `Unsupported function name: "shift_left"`.
   
   ## What changes are included in this PR?
   
   - `name_to_op` maps `shift_left` -> `BitwiseShiftLeft` and `shift_right` -> 
`BitwiseShiftRight`.
   - The existing `bitwise_shift_left` / `bitwise_shift_right` names still 
work, so plans written by current and older DataFusion versions keep loading.
   
   Things I deliberately left out, happy to follow up if wanted:
   
   - The producer still writes `bitwise_shift_*`. Switching it to the standard 
names would make plans from a new DataFusion unreadable by older DataFusion 
consumers, so it seemed better as a separate change.
   - `shift_right_unsigned` is not mapped. DataFusion's `>>` is an arithmetic 
shift for signed integers (matching Substrait's `shift_right`), and I don't 
think there is an existing logical-shift operator to map it to.
   - One thing I'm not fully sure about: Substrait leaves the result of a shift 
amount outside the bit width up to the implementation, and DataFusion wraps it. 
I didn't change that behaviour here.
   
   ## What is the testing strategy for this PR?
   
   Added `test_shift_function_names` in `consumer/expr/scalar_function.rs`, 
which consumes a scalar function for each of the four names (standard and 
legacy) and checks the resulting expression is the expected `<<` / `>>` binary 
expression.
   
   Locally: `cargo test -p datafusion-substrait` passes, and `cargo clippy -p 
datafusion-substrait --all-targets -- -D warnings` and `cargo fmt --check` are 
clean.
   
   ## Are there any user-facing changes?
   
   The Substrait consumer now accepts plans that use the standard shift 
function names. No API changes.


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

Reply via email to