lyne7-sc commented on code in PR #22591:
URL: https://github.com/apache/datafusion/pull/22591#discussion_r3331356514
##########
datafusion/physical-expr/src/expressions/binary.rs:
##########
@@ -177,82 +177,97 @@ fn is_date_minus_date(lhs: &DataType, rhs: &DataType) ->
bool {
)
}
-/// Computes the difference between two dates and returns the result as Int64
(days)
-/// This aligns with PostgreSQL, DuckDB, and MySQL behavior where date - date
returns an integer
+/// Milliseconds per day, used for Date64 subtraction.
+const MILLIS_PER_DAY: i64 = 86_400_000;
+
+/// Evaluates `Date32 - Date32` or `Date64 - Date64`, returning the difference
in
+/// whole days as `Int64`.
///
-/// Implementation: Uses Arrow's sub_wrapping to get Duration, then converts
to Int64 days
+/// This matches the behavior of PostgreSQL, DuckDB, and MySQL, where
+/// `date - date` yields an integer day count rather than an interval.
fn apply_date_subtraction(
lhs: &ColumnarValue,
rhs: &ColumnarValue,
) -> Result<ColumnarValue> {
- use arrow::compute::kernels::numeric::sub_wrapping;
-
- // Use Arrow's sub_wrapping to compute the Duration result
- let duration_result = apply(lhs, rhs, sub_wrapping)?;
-
- // Convert Duration to Int64 (days)
- match duration_result {
- ColumnarValue::Array(array) => {
- let int64_array = duration_to_days(&array)?;
- Ok(ColumnarValue::Array(int64_array))
+ match (lhs.data_type(), rhs.data_type()) {
+ (DataType::Date32, DataType::Date32) => {
+ subtract_date_to_days::<Date32Type>(lhs, rhs, |l, r| {
+ i64::from(l) - i64::from(r)
+ })
}
- ColumnarValue::Scalar(scalar) => {
- // Convert scalar Duration to Int64 days
- let array = scalar.to_array_of_size(1)?;
- let int64_array = duration_to_days(&array)?;
- let int64_scalar =
ScalarValue::try_from_array(int64_array.as_ref(), 0)?;
- Ok(ColumnarValue::Scalar(int64_scalar))
+ (DataType::Date64, DataType::Date64) => {
+ subtract_date_to_days::<Date64Type>(lhs, rhs, |l, r| {
+ l.wrapping_sub(r) / MILLIS_PER_DAY
+ })
}
+ (_, _) => unreachable!("apply_date_subtraction called with non-date
types"),
}
}
-/// Converts a Duration array to Int64 days
-/// Handles different Duration time units (Second, Millisecond, Microsecond,
Nanosecond)
-fn duration_to_days(array: &ArrayRef) -> Result<ArrayRef> {
- use datafusion_common::cast::{
- as_duration_microsecond_array, as_duration_millisecond_array,
- as_duration_nanosecond_array, as_duration_second_array,
- };
+/// Generic date subtraction: operates directly on the native primitive values
+/// of `T` (i32 for Date32, i64 for Date64), applying `day_diff_fn` to produce
+/// an Int64 day count.
+fn subtract_date_to_days<T: ArrowPrimitiveType>(
+ lhs: &ColumnarValue,
+ rhs: &ColumnarValue,
+ day_diff_fn: impl Fn(T::Native, T::Native) -> i64,
+) -> Result<ColumnarValue>
+where
+ T::Native: Copy,
+{
+ /// Extract the native value from a scalar by converting to a
single-element
+ /// array and reading index 0. Returns `None` for null scalars.
+ fn scalar_to_native<P: ArrowPrimitiveType>(
Review Comment:
Make senses, I tried switching this to destructure directly. The added
complexity looks manageable to me.
--
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]
