tustvold commented on code in PR #6778:
URL: https://github.com/apache/arrow-datafusion/pull/6778#discussion_r1243811907
##########
datafusion/expr/src/type_coercion/binary.rs:
##########
@@ -66,185 +193,64 @@ fn mathematics_temporal_result_type(
| (Timestamp(Nanosecond, _), Timestamp(Nanosecond, _)) => {
Some(Interval(MonthDayNano))
}
- (Timestamp(_, _), Timestamp(_, _)) => None,
// date - date
(Date32, Date32) => Some(Interval(DayTime)),
(Date64, Date64) => Some(Interval(MonthDayNano)),
- (Date32, Date64) | (Date64, Date32) => Some(Interval(MonthDayNano)),
- // date - timestamp, timestamp - date
- (Date32, Timestamp(_, _))
- | (Timestamp(_, _), Date32)
- | (Date64, Timestamp(_, _))
- | (Timestamp(_, _), Date64) => {
- // TODO: make get_result_type must after coerce type.
- // if type isn't coerced, we need get common type, and then get
result type.
- let common_type = temporal_coercion(lhs_type, rhs_type);
- common_type.and_then(|t| mathematics_temporal_result_type(&t, &t))
- }
_ => None,
}
}
/// returns the resulting type of a binary expression evaluating the `op` with
the left and right hand types
pub fn get_result_type(
- lhs_type: &DataType,
+ lhs: &DataType,
op: &Operator,
- rhs_type: &DataType,
+ rhs: &DataType,
) -> Result<DataType> {
- if op.is_numerical_operators() && any_decimal(lhs_type, rhs_type) {
- let (coerced_lhs_type, coerced_rhs_type) =
- math_decimal_coercion(lhs_type, rhs_type);
-
- let lhs_type = coerced_lhs_type.unwrap_or(lhs_type.clone());
- let rhs_type = coerced_rhs_type.unwrap_or(rhs_type.clone());
-
- if op.is_numerical_operators() {
- if let Some(result_type) =
- decimal_op_mathematics_type(op, &lhs_type, &rhs_type)
- {
- return Ok(result_type);
- }
- }
- }
- let result = match op {
- Operator::And
- | Operator::Or
- | Operator::Eq
- | Operator::NotEq
- | Operator::Lt
- | Operator::Gt
- | Operator::GtEq
- | Operator::LtEq
- | Operator::RegexMatch
- | Operator::RegexIMatch
- | Operator::RegexNotMatch
- | Operator::RegexNotIMatch
- | Operator::IsDistinctFrom
- | Operator::IsNotDistinctFrom => Some(DataType::Boolean),
- Operator::Plus | Operator::Minus
- if is_datetime(lhs_type) && is_datetime(rhs_type)
- || (is_interval(lhs_type) && is_interval(rhs_type))
- || (is_datetime(lhs_type) && is_interval(rhs_type))
- || (is_interval(lhs_type) && is_datetime(rhs_type)) =>
- {
- mathematics_temporal_result_type(lhs_type, rhs_type)
- }
- // following same with `coerce_types`
- Operator::BitwiseAnd
- | Operator::BitwiseOr
- | Operator::BitwiseXor
- | Operator::BitwiseShiftRight
- | Operator::BitwiseShiftLeft => bitwise_coercion(lhs_type, rhs_type),
- Operator::Plus
- | Operator::Minus
- | Operator::Modulo
- | Operator::Divide
- | Operator::Multiply => mathematics_numerical_coercion(lhs_type,
rhs_type),
- Operator::StringConcat => string_concat_coercion(lhs_type, rhs_type),
- };
-
- result.ok_or(DataFusionError::Plan(format!(
- "Unsupported argument types. Can not evaluate {lhs_type:?} {op}
{rhs_type:?}"
- )))
+ signature(lhs, op, rhs).map(|sig| sig.ret)
}
-/// Coercion rules for all binary operators. Returns the 'coerce_types'
-/// is returns the type the arguments should be coerced to
-///
-/// Returns None if no suitable type can be found.
-pub fn coerce_types(
- lhs_type: &DataType,
+/// Returns the coerced input types for a binary expression evaluating the
`op` with the left and right hand types
+pub fn get_input_types(
+ lhs: &DataType,
op: &Operator,
- rhs_type: &DataType,
-) -> Result<DataType> {
- // This result MUST be compatible with `binary_coerce`
- let result = match op {
- Operator::BitwiseAnd
- | Operator::BitwiseOr
- | Operator::BitwiseXor
- | Operator::BitwiseShiftRight
- | Operator::BitwiseShiftLeft => bitwise_coercion(lhs_type, rhs_type),
- Operator::And | Operator::Or => match (lhs_type, rhs_type) {
- // logical binary boolean operators can only be evaluated in bools
or nulls
- (DataType::Boolean, DataType::Boolean)
- | (DataType::Null, DataType::Null)
- | (DataType::Boolean, DataType::Null)
- | (DataType::Null, DataType::Boolean) => Some(DataType::Boolean),
- _ => None,
- },
- // logical comparison operators have their own rules, and always
return a boolean
- Operator::Eq
- | Operator::NotEq
- | Operator::Lt
- | Operator::Gt
- | Operator::GtEq
- | Operator::LtEq
- | Operator::IsDistinctFrom
- | Operator::IsNotDistinctFrom => comparison_coercion(lhs_type,
rhs_type),
- Operator::Plus | Operator::Minus
- if is_interval(lhs_type) && is_interval(rhs_type) =>
- {
- temporal_coercion(lhs_type, rhs_type)
- }
- Operator::Minus if is_datetime(lhs_type) && is_datetime(rhs_type) => {
- temporal_coercion(lhs_type, rhs_type)
- }
- // for math expressions, the final value of the coercion is also the
return type
- // because coercion favours higher information types
- Operator::Plus
- | Operator::Minus
- | Operator::Modulo
- | Operator::Divide
- | Operator::Multiply => mathematics_numerical_coercion(lhs_type,
rhs_type),
- Operator::RegexMatch
- | Operator::RegexIMatch
- | Operator::RegexNotMatch
- | Operator::RegexNotIMatch => regex_coercion(lhs_type, rhs_type),
- // "||" operator has its own rules, and always return a string type
- Operator::StringConcat => string_concat_coercion(lhs_type, rhs_type),
- };
-
- // re-write the error message of failed coercions to include the
operator's information
- result.ok_or(DataFusionError::Plan(format!("{lhs_type:?} {op} {rhs_type:?}
can't be evaluated because there isn't a common type to coerce the types to")))
+ rhs: &DataType,
+) -> Result<(DataType, DataType)> {
+ signature(lhs, op, rhs).map(|sig| (sig.lhs, sig.rhs))
}
/// Coercion rules for mathematics operators between decimal and non-decimal
types.
pub fn math_decimal_coercion(
lhs_type: &DataType,
rhs_type: &DataType,
-) -> (Option<DataType>, Option<DataType>) {
+) -> Option<(DataType, DataType)> {
Review Comment:
I updated this method to be more consistent with the signature of other
coercion functions, i.e. returning `None` to indicate a coercion failure,
instead of don't coerce
--
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]
