tustvold commented on code in PR #6778:
URL: https://github.com/apache/arrow-datafusion/pull/6778#discussion_r1243809317
##########
datafusion/expr/src/type_coercion/binary.rs:
##########
@@ -25,9 +25,144 @@ use arrow::datatypes::{
use datafusion_common::DataFusionError;
use datafusion_common::Result;
-use crate::type_coercion::{is_datetime, is_decimal, is_interval, is_numeric};
+use crate::type_coercion::{is_decimal, is_numeric};
use crate::Operator;
+/// The type signature of an instantiation of binary expression
+struct Signature {
+ /// The type to coerce the left argument to
+ lhs: DataType,
+ /// The type to coerce the right argument to
+ rhs: DataType,
+ /// The return type of the expression
+ ret: DataType,
+}
+
+impl Signature {
+ /// A signature where the inputs are coerced to the same type as the output
+ fn coerced(t: DataType) -> Self {
+ Self {
+ lhs: t.clone(),
+ rhs: t.clone(),
+ ret: t,
+ }
+ }
+
+ /// A signature where the inputs are coerced to the same type with a
boolean output
+ fn comparison(t: DataType) -> Self {
+ Self {
+ lhs: t.clone(),
+ rhs: t,
+ ret: DataType::Boolean,
+ }
+ }
+}
+
+/// Returns a [`Signature`] for applying `op` to arguments of type `lhs` and
`rhs`
+fn signature(lhs: &DataType, op: &Operator, rhs: &DataType) ->
Result<Signature> {
+ match op {
+ Operator::Eq |
+ Operator::NotEq |
+ Operator::Lt |
+ Operator::LtEq |
+ Operator::Gt |
+ Operator::GtEq |
+ Operator::IsDistinctFrom |
+ Operator::IsNotDistinctFrom => {
+ comparison_coercion(lhs,
rhs).map(Signature::comparison).ok_or_else(|| {
+ DataFusionError::Plan(format!(
+ "Cannot infer common argument type for comparison
operation {lhs} {op} {rhs}"
+ ))
+ })
+ }
+ Operator::And | Operator::Or => match (lhs, rhs) {
+ // 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) =>
Ok(Signature::coerced(DataType::Boolean)),
+ _ => Err(DataFusionError::Plan(format!(
+ "Cannot infer common argument type for logical boolean
operation {lhs} {op} {rhs}"
+ ))),
+ },
+ Operator::RegexMatch |
+ Operator::RegexIMatch |
+ Operator::RegexNotMatch |
+ Operator::RegexNotIMatch => {
+ regex_coercion(lhs, rhs).map(Signature::comparison).ok_or_else(|| {
+ DataFusionError::Plan(format!(
+ "Cannot infer common argument type for regex operation
{lhs} {op} {rhs}"
+ ))
+ })
+ }
+ Operator::BitwiseAnd
+ | Operator::BitwiseOr
+ | Operator::BitwiseXor
+ | Operator::BitwiseShiftRight
+ | Operator::BitwiseShiftLeft => {
+ bitwise_coercion(lhs, rhs).map(Signature::coerced).ok_or_else(|| {
+ DataFusionError::Plan(format!(
+ "Cannot infer common type for bitwise operation {lhs} {op}
{rhs}"
+ ))
+ })
+ }
+ Operator::StringConcat => {
+ string_concat_coercion(lhs,
rhs).map(Signature::coerced).ok_or_else(|| {
+ DataFusionError::Plan(format!(
+ "Cannot infer common string type for string concat
operation {lhs} {op} {rhs}"
+ ))
+ })
+ }
+ Operator::Plus |
+ Operator::Minus |
+ Operator::Multiply |
+ Operator::Divide|
+ Operator::Modulo => {
+ // TODO: this logic would be easier to follow if the functions
were inlined
Review Comment:
I plan to do this a follow up PR, but wanted to avoid conflating too many
things in one PR
--
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]
