alamb commented on code in PR #3676:
URL: https://github.com/apache/arrow-datafusion/pull/3676#discussion_r985224988
##########
datafusion/optimizer/src/type_coercion.rs:
##########
@@ -410,6 +454,27 @@ fn coerce_arguments_for_signature(
.collect::<Result<Vec<_>>>()
}
+/// Attempts to coerce the types of `then_types` to be comparable with the
+/// `else_type`.
Review Comment:
```suggestion
/// Find a common coerceable type for all `then_types` as well
/// and the `else_type`, if specified.
```
##########
datafusion/core/tests/sql/expr.rs:
##########
@@ -152,25 +152,25 @@ async fn case_expr_with_null() -> Result<()> {
let actual = execute_to_batches(&ctx, sql).await;
let expected = vec![
- "+------------------------------------------------+",
- "| CASE WHEN #a.b IS NULL THEN NULL ELSE #a.b END |",
- "+------------------------------------------------+",
- "| |",
- "| 3 |",
- "+------------------------------------------------+",
+ "+----------------------------------------------+",
+ "| CASE WHEN a.b IS NULL THEN NULL ELSE a.b END |",
Review Comment:
this looks like an improvement but I don't understand the change
##########
datafusion/optimizer/src/type_coercion.rs:
##########
@@ -354,6 +354,50 @@ impl ExprRewriter for TypeCoercionRewriter {
}
}
}
+ Expr::Case {
+ expr,
+ when_then_expr,
+ else_expr,
+ } => {
+ // all the result of then and else should be convert to a
common data type,
+ // if they can be coercible to a common data type, return
error.
+ let then_types = when_then_expr
+ .iter()
+ .map(|when_then| when_then.1.get_type(&self.schema))
+ .collect::<Result<Vec<_>>>()?;
+ let else_type = match &else_expr {
+ None => Ok(None),
+ Some(expr) => expr.get_type(&self.schema).map(Some),
+ }?;
+ let case_when_coerce_type =
+ get_coerce_type_for_case_when(&then_types, &else_type);
+ match case_when_coerce_type {
+ None => Err(DataFusionError::Internal(format!(
+ "Failed to coerce types {:?} and {:?} in CASE WHEN
expression",
+ then_types, else_type
+ ))),
Review Comment:
```suggestion
None => Err(DataFusionError::Plan(format!(
"Failed to coerce then ({:?}) and else ({:?}) to
common types in CASE WHEN expression",
then_types, else_type
))),
```
##########
datafusion/optimizer/src/type_coercion.rs:
##########
@@ -410,6 +454,27 @@ fn coerce_arguments_for_signature(
.collect::<Result<Vec<_>>>()
}
+/// Attempts to coerce the types of `then_types` to be comparable with the
+/// `else_type`.
+/// Returns the common data type for `then_types` and `else_type`
+fn get_coerce_type_for_case_when(
+ then_types: &[DataType],
+ else_type: &Option<DataType>,
+) -> Option<DataType> {
+ let else_type = match else_type {
+ None => then_types[0].clone(),
+ Some(data_type) => data_type.clone(),
+ };
+ then_types
+ .iter()
+ .fold(Some(else_type), |left, right_type| match left {
+ None => None,
Review Comment:
```suggestion
// failed to find a valid coercion in a previous iteration
None => None,
```
--
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]
