alamb commented on code in PR #6785:
URL: https://github.com/apache/arrow-datafusion/pull/6785#discussion_r1246892438
##########
datafusion/physical-expr/src/expressions/binary.rs:
##########
@@ -2968,16 +2962,10 @@ mod tests {
op: Operator,
expected: BooleanArray,
) -> Result<()> {
- let left_type = left.data_type();
- let right_type = right.data_type();
- let (lhs, rhs) = get_input_types(left_type, &op, right_type)?;
-
- let left_expr = try_cast(col("a", schema)?, schema, lhs)?;
- let right_expr = try_cast(col("b", schema)?, schema, rhs)?;
- let arithmetic_op = binary_simple(left_expr, op, right_expr, schema);
+ let op = binary_op(col("a", schema)?, op, col("b", schema)?, schema)?;
Review Comment:
this is a much nicer formulation
##########
datafusion/expr/src/type_coercion/binary.rs:
##########
@@ -226,13 +226,13 @@ fn math_decimal_coercion(
use arrow::datatypes::DataType::*;
match (lhs_type, rhs_type) {
- (Dictionary(key_type, value_type), _) => {
+ (Dictionary(_, value_type), _) => {
Review Comment:
maybe it is worth a comment here explaining we are expanding out
dictionaries to the underlying type and leave a comment pointing back at this
PR / a summary of
https://github.com/apache/arrow-datafusion/pull/6785#issuecomment-1612957624
I think that is critical context
--
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]
