andygrove commented on code in PR #6458:
URL: https://github.com/apache/datafusion-comet/pull/6458#discussion_r4149555706
##########
native/core/src/execution/planner.rs:
##########
@@ -802,8 +803,35 @@ impl PhysicalPlanner {
let true_expr =
self.create_expr(expr.true_expr.as_ref().unwrap(),
Arc::clone(&input_schema))?;
let false_expr =
- self.create_expr(expr.false_expr.as_ref().unwrap(),
input_schema)?;
- Ok(Arc::new(IfExpr::new(if_expr, true_expr, false_expr)))
+ self.create_expr(expr.false_expr.as_ref().unwrap(),
Arc::clone(&input_schema))?;
+ // Spark adds no cast when the branches differ only in whether
a nested field can
+ // be NULL, but `IfExpr` reports the THEN branch's type and
returns the ELSE
+ // branch's array unchanged when no row of a batch takes the
THEN branch. So cast a
+ // branch whose type differs from the common type, as
`create_case_expr` does for
+ // CASE WHEN. The THEN branch goes first so that the common
type keeps its field
+ // names, as Spark's `If` does.
+ let true_type = true_expr.data_type(&input_schema)?;
+ let false_type = false_expr.data_type(&input_schema)?;
+ let common_type = type_union_coercion(&true_type, &false_type);
Review Comment:
Done in 171e50ec18. `create_if_expr` now sits next to `create_case_when` in
`case_when.rs`, with the let-else shape, and the If arm in the planner is a
single call. The cast is shared through a small `coerce_branch` helper. I kept
IF on its own coercion rather than calling `create_case_when`, because that one
starts from the ELSE branch and would give IF the ELSE branch's field names.
`if_reconciles_timestamp_timezone_labels` covers the label path, and
`if_reconciles_struct_field_nullability_and_names` covers nullability both ways
and a name that differs only in case. Both evaluate mixed, all-THEN and
all-ELSE batches and check the result type each time, and both fail with the
casts disabled.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
