2010YOUY01 commented on code in PR #13953: URL: https://github.com/apache/datafusion/pull/13953#discussion_r1900547926
########## datafusion/physical-expr/src/expressions/case.rs: ########## @@ -394,6 +401,43 @@ impl CaseExpr { Ok(ColumnarValue::Array(zip(&when_value, &then_value, &else_)?)) } + + fn expr_or_expr(&self, batch: &RecordBatch) -> Result<ColumnarValue> { + let return_type = self.data_type(&batch.schema())?; + + // evalute when condition on batch + let when_value = self.when_then_expr[0].0.evaluate(batch)?; + let when_value = when_value.into_array(batch.num_rows())?; + let when_value = as_boolean_array(&when_value).map_err(|e| { + DataFusionError::Context( + "WHEN expression did not return a BooleanArray".to_string(), + Box::new(e), + ) Review Comment: ```suggestion internal_datafusion_err!("WHEN expression did not return a BooleanArray") ``` nit: We can assume all type checks have been done before, then inside this function all cast failures should be unreachable, so we can use internal error instead ########## datafusion/physical-expr/src/expressions/case.rs: ########## @@ -394,6 +401,43 @@ impl CaseExpr { Ok(ColumnarValue::Array(zip(&when_value, &then_value, &else_)?)) } + + fn expr_or_expr(&self, batch: &RecordBatch) -> Result<ColumnarValue> { + let return_type = self.data_type(&batch.schema())?; + + // evalute when condition on batch + let when_value = self.when_then_expr[0].0.evaluate(batch)?; + let when_value = when_value.into_array(batch.num_rows())?; + let when_value = as_boolean_array(&when_value).map_err(|e| { + DataFusionError::Context( + "WHEN expression did not return a BooleanArray".to_string(), + Box::new(e), + ) + })?; + + // Treat 'NULL' as false value + let when_value = match when_value.null_count() { + 0 => Cow::Borrowed(when_value), + _ => Cow::Owned(prep_null_mask_filter(when_value)), + }; + + let then_value = self.when_then_expr[0] + .1 + .evaluate_selection(batch, &when_value)? + .into_array(batch.num_rows())?; + + // evaluate else expression on the values not covered by when_value + let remainder = not(&when_value)?; + let e = self.else_expr.as_ref().unwrap(); + // keep `else_expr`'s data type and return type consistent + let expr = try_cast(Arc::clone(e), &batch.schema(), return_type.clone()) + .unwrap_or_else(|_| Arc::clone(e)); Review Comment: Here is similar, we can return an internal error directly, and avoid propagating the casting failure -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org