alamb commented on code in PR #18525:
URL: https://github.com/apache/datafusion/pull/18525#discussion_r2505633207
##########
datafusion/functions/src/utils.rs:
##########
@@ -140,38 +140,25 @@ where
F: Fn(L::Native, R::Native) -> Result<O::Native, ArrowError>,
R::Native: TryFrom<ScalarValue>,
{
- Ok(match right {
+ let left = left.as_primitive::<L>();
+ let right = right.cast_to(&R::DATA_TYPE, None)?;
Review Comment:
I wonder why we aren't also casting left as well up here (it was only done
in the branch below 🤔 )
##########
datafusion/functions/src/utils.rs:
##########
@@ -140,38 +140,25 @@ where
F: Fn(L::Native, R::Native) -> Result<O::Native, ArrowError>,
R::Native: TryFrom<ScalarValue>,
{
- Ok(match right {
+ let left = left.as_primitive::<L>();
+ let right = right.cast_to(&R::DATA_TYPE, None)?;
+ let result = match right {
ColumnarValue::Scalar(scalar) => {
- let right_value: R::Native =
- R::Native::try_from(scalar.clone()).map_err(|_| {
- DataFusionError::NotImplemented(format!(
- "Cannot convert scalar value {} to {}",
- &scalar,
- R::DATA_TYPE
- ))
- })?;
- let left_array = left.as_primitive::<L>();
- // Bind right value
- let result =
- left_array.try_unary::<_, O, _>(|lvalue| fun(lvalue,
right_value))?;
- Arc::new(result) as _
+ let right = R::Native::try_from(scalar.clone()).map_err(|_| {
+ DataFusionError::NotImplemented(format!(
+ "Cannot convert scalar value {} to {}",
+ &scalar,
+ R::DATA_TYPE
+ ))
+ })?;
+ left.try_unary::<_, O, _>(|lvalue| fun(lvalue, right))?
}
ColumnarValue::Array(right) => {
- let right_casted = arrow::compute::cast(&right, &R::DATA_TYPE)?;
- let right_array = right_casted.as_primitive::<R>();
-
- // Types are compatible even they are decimals with different
scale or precision
- let result = if PrimitiveArray::<L>::is_compatible(&L::DATA_TYPE) {
Review Comment:
The documentation of
https://docs.rs/arrow/latest/arrow/array/struct.PrimitiveArray.html#method.is_compatible
says
> This is equivalent to data_type == T::DATA_TYPE, however ignores timestamp
timezones and decimal precision and scale
So maybe it is something related to timezones?
--
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]
