This is an automated email from the ASF dual-hosted git repository.
jakevin pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-datafusion.git
The following commit(s) were added to refs/heads/main by this push:
new 93771052c5 fix: add condition check for BinaryExpr Date/Time +/-
Interval (#5932)
93771052c5 is described below
commit 93771052c5ac31f2cf22b8c25bf938656afe1047
Author: jakevin <[email protected]>
AuthorDate: Tue Apr 11 17:29:13 2023 +0800
fix: add condition check for BinaryExpr Date/Time +/- Interval (#5932)
---
.../tests/sqllogictests/test_files/timestamps.slt | 4 +-
datafusion/expr/src/type_coercion/binary.rs | 46 +++++++++++-----------
datafusion/optimizer/src/analyzer/type_coercion.rs | 11 +++---
3 files changed, 31 insertions(+), 30 deletions(-)
diff --git a/datafusion/core/tests/sqllogictests/test_files/timestamps.slt
b/datafusion/core/tests/sqllogictests/test_files/timestamps.slt
index 3bea8cb6c9..3887fc84ad 100644
--- a/datafusion/core/tests/sqllogictests/test_files/timestamps.slt
+++ b/datafusion/core/tests/sqllogictests/test_files/timestamps.slt
@@ -656,7 +656,7 @@ SELECT ts1 + i FROM foo;
2003-07-12T01:31:15.000123463
# Timestamp + Timestamp => error
-statement error DataFusion error: Error during planning:
Timestamp\(Nanosecond, None\) Timestamp\(Nanosecond, None\) is an unsupported
operation. addition/subtraction on dates/timestamps only supported with
interval types
+statement error DataFusion error: Error during planning:
Timestamp\(Nanosecond, None\) \+ Timestamp\(Nanosecond, None\) can't be
evaluated because there isn't a common type to coerce the types to
SELECT ts1 + ts2
FROM foo;
@@ -690,4 +690,4 @@ SELECT
'2000-01-01T00:00:00'::timestamp::timestamptz <=
'2000-01-01T00:00:00'::timestamp
;
----
-true false true true
\ No newline at end of file
+true false true true
diff --git a/datafusion/expr/src/type_coercion/binary.rs
b/datafusion/expr/src/type_coercion/binary.rs
index 99a5efb4fd..33ec5e4365 100644
--- a/datafusion/expr/src/type_coercion/binary.rs
+++ b/datafusion/expr/src/type_coercion/binary.rs
@@ -125,7 +125,7 @@ pub fn coerce_types(
|| !is_timestamp(rhs_type)
|| *op != Operator::Minus) =>
{
- temporal_add_sub_coercion(lhs_type, rhs_type, op)?
+ temporal_add_sub_coercion(lhs_type, rhs_type, op)
}
// for math expressions, the final value of the coercion is also the
return type
// because coercion favours higher information types
@@ -217,29 +217,33 @@ pub fn temporal_add_sub_coercion(
lhs_type: &DataType,
rhs_type: &DataType,
op: &Operator,
-) -> Result<Option<DataType>> {
+) -> Option<DataType> {
match (lhs_type, rhs_type, op) {
// if an interval is being added/subtracted from a date/timestamp,
return the date/timestamp data type
(lhs, rhs, _) if is_interval(lhs) && (is_date(rhs) ||
is_timestamp(rhs)) => {
- Ok(Some(rhs.clone()))
+ Some(rhs.clone())
}
(lhs, rhs, _) if is_interval(rhs) && (is_date(lhs) ||
is_timestamp(lhs)) => {
- Ok(Some(lhs.clone()))
+ Some(lhs.clone())
}
// if two timestamps are being subtracted, check their time units and
return the corresponding interval data type
(lhs, rhs, Operator::Minus) if is_timestamp(lhs) && is_timestamp(rhs)
=> {
handle_timestamp_minus(lhs, rhs)
}
// if two intervals are being added/subtracted, check their interval
units and return the corresponding interval data type
- (lhs, rhs, _) if is_interval(lhs) && is_interval(rhs) =>
handle_interval_addition(lhs, rhs),
+ (lhs, rhs, _) if is_interval(lhs) && is_interval(rhs) => {
+ handle_interval_addition(lhs, rhs)
+ }
// if two date/timestamp are being added/subtracted, return an error
indicating that the operation is not supported
- (lhs, rhs, _) if (is_date(lhs) || is_timestamp(lhs)) && (is_date(rhs)
|| is_timestamp(rhs)) => {
- Err(DataFusionError::Plan(format!(
- "{lhs_type:?} {rhs_type:?} is an unsupported operation.
addition/subtraction on dates/timestamps only supported with interval types"
- )))
+ (lhs, rhs, Operator::Minus)
+ if (is_date(lhs) || is_timestamp(lhs))
+ && (is_date(rhs) || is_timestamp(rhs)) =>
+ {
+ // TODO: support Minus
+ None
}
// return None if no coercion is possible
- _ => Ok(None),
+ _ => None,
}
}
@@ -247,19 +251,19 @@ pub fn temporal_add_sub_coercion(
// representing the sum of them. If the two interval data types have different
units, it returns an interval data type
// with "IntervalUnit::MonthDayNano". If the two interval data types are
already "IntervalUnit::YearMonth" or "IntervalUnit::DayTime",
// it returns an interval data type with the same unit as the operands.
-fn handle_interval_addition(lhs: &DataType, rhs: &DataType) ->
Result<Option<DataType>> {
+fn handle_interval_addition(lhs: &DataType, rhs: &DataType) ->
Option<DataType> {
match (lhs, rhs) {
// operation with the same types
(
DataType::Interval(IntervalUnit::YearMonth),
DataType::Interval(IntervalUnit::YearMonth),
- ) => Ok(Some(DataType::Interval(IntervalUnit::YearMonth))),
+ ) => Some(DataType::Interval(IntervalUnit::YearMonth)),
(
DataType::Interval(IntervalUnit::DayTime),
DataType::Interval(IntervalUnit::DayTime),
- ) => Ok(Some(DataType::Interval(IntervalUnit::DayTime))),
+ ) => Some(DataType::Interval(IntervalUnit::DayTime)),
// operation with MonthDayNano's or different types
- (_, _) => Ok(Some(DataType::Interval(IntervalUnit::MonthDayNano))),
+ (_, _) => Some(DataType::Interval(IntervalUnit::MonthDayNano)),
}
}
@@ -267,7 +271,7 @@ fn handle_interval_addition(lhs: &DataType, rhs: &DataType)
-> Result<Option<Dat
// representing the difference between them, either "IntervalUnit::DayTime" if
the time unit is second or millisecond,
// or "IntervalUnit::MonthDayNano" if the time unit is microsecond or
nanosecond. If the two timestamp data types have
// different time units, it returns an error indicating that "The timestamps
have different types".
-fn handle_timestamp_minus(lhs: &DataType, rhs: &DataType) ->
Result<Option<DataType>> {
+fn handle_timestamp_minus(lhs: &DataType, rhs: &DataType) -> Option<DataType> {
match (lhs, rhs) {
(
DataType::Timestamp(TimeUnit::Second, _),
@@ -276,7 +280,7 @@ fn handle_timestamp_minus(lhs: &DataType, rhs: &DataType)
-> Result<Option<DataT
| (
DataType::Timestamp(TimeUnit::Millisecond, _),
DataType::Timestamp(TimeUnit::Millisecond, _),
- ) => Ok(Some(DataType::Interval(IntervalUnit::DayTime))),
+ ) => Some(DataType::Interval(IntervalUnit::DayTime)),
(
DataType::Timestamp(TimeUnit::Microsecond, _),
DataType::Timestamp(TimeUnit::Microsecond, _),
@@ -284,10 +288,8 @@ fn handle_timestamp_minus(lhs: &DataType, rhs: &DataType)
-> Result<Option<DataT
| (
DataType::Timestamp(TimeUnit::Nanosecond, _),
DataType::Timestamp(TimeUnit::Nanosecond, _),
- ) => Ok(Some(DataType::Interval(IntervalUnit::MonthDayNano))),
- (_, _) => Err(DataFusionError::Plan(
- "The timestamps have different types".to_string(),
- )),
+ ) => Some(DataType::Interval(IntervalUnit::MonthDayNano)),
+ (_, _) => None,
}
}
@@ -1003,12 +1005,12 @@ mod tests {
)
.unwrap_err()
.to_string();
- assert_contains!(&err, "The timestamps have different types");
+ assert_contains!(&err, "Timestamp(Nanosecond, None) -
Timestamp(Millisecond, None) can't be evaluated because there isn't a common
type to coerce the types to");
let err = coerce_types(&DataType::Date32, &Operator::Plus,
&DataType::Date64)
.unwrap_err()
.to_string();
- assert_contains!(&err, "Date32 Date64 is an unsupported operation.
addition/subtraction on dates/timestamps only supported with interval types");
+ assert_contains!(&err, "Date32 + Date64 can't be evaluated because
there isn't a common type to coerce the types to");
Ok(())
}
diff --git a/datafusion/optimizer/src/analyzer/type_coercion.rs
b/datafusion/optimizer/src/analyzer/type_coercion.rs
index 952cc04b0b..9a210601f6 100644
--- a/datafusion/optimizer/src/analyzer/type_coercion.rs
+++ b/datafusion/optimizer/src/analyzer/type_coercion.rs
@@ -242,20 +242,19 @@ impl TreeNodeRewriter for TypeCoercionRewriter {
op,
ref right,
}) => {
+ // this is a workaround for
https://github.com/apache/arrow-datafusion/issues/3419
let left_type = left.get_type(&self.schema)?;
let right_type = right.get_type(&self.schema)?;
match (&left_type, &right_type) {
+ // Handle some case about Interval.
(
DataType::Date32 | DataType::Date64 |
DataType::Timestamp(_, _),
&DataType::Interval(_),
- )
- | (
+ ) if matches!(op, Operator::Plus | Operator::Minus) =>
Ok(expr),
+ (
&DataType::Interval(_),
DataType::Date32 | DataType::Date64 |
DataType::Timestamp(_, _),
- ) => {
- // this is a workaround for
https://github.com/apache/arrow-datafusion/issues/3419
- Ok(expr.clone())
- }
+ ) if matches!(op, Operator::Plus) => Ok(expr),
(DataType::Timestamp(_, _), DataType::Timestamp(_, _))
if op.is_numerical_operators() =>
{
