jorgecarleitao commented on a change in pull request #7998:
URL: https://github.com/apache/arrow/pull/7998#discussion_r472628286
##########
File path: rust/datafusion/src/execution/physical_plan/expressions.rs
##########
@@ -966,11 +966,32 @@ impl fmt::Display for BinaryExpr {
impl PhysicalExpr for BinaryExpr {
fn data_type(&self, input_schema: &Schema) -> Result<DataType> {
- self.left.data_type(input_schema)
+ Ok(match self.op {
+ Operator::And
+ | Operator::Or
+ | Operator::Not
+ | Operator::NotLike
+ | Operator::Like
+ | Operator::Lt
+ | Operator::LtEq
+ | Operator::Eq
+ | Operator::NotEq
+ | Operator::Gt
+ | Operator::GtEq => DataType::Boolean,
+ Operator::Plus
+ | Operator::Minus
+ | Operator::Multiply
+ | Operator::Divide
+ | Operator::Modulus => {
+ // this assumes that the left and right expressions have
already been co-coerced
+ // to the same type
+ self.left.data_type(input_schema)?
Review comment:
Yes.
In another PR (now closed), what I did was to move this to a new function
that accepts both a physical or logical expressions (a trait object
`DataTyped`), and returns the correct type based on either of them. I.e.
```
fn binary_op_type(lhs: DataTyped, rhs: DataTyped, schema) ->
Result<DataType>;
```
where `DataTyped` is a trait of a single method, `fn data_type(schema) ->
Result<DataType>`. This way we do not have to keep track of these in 2
different places in the code.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
