sunchao commented on code in PR #6763:
URL: https://github.com/apache/datafusion-comet/pull/6763#discussion_r4212107352
##########
native/spark-expr/src/conditional_funcs/case_when.rs:
##########
@@ -349,6 +350,13 @@ impl PhysicalExpr for CaseWhenExpr {
}
fn evaluate(&self, batch: &RecordBatch) -> Result<ColumnarValue> {
+ // No row chooses a branch, which both the eager and the lazy
evaluation answer with a
+ // scalar NULL. A scalar from an empty batch is taken to mean the
expression is constant,
+ // as IN does to build its list once, so return an empty array instead.
+ if batch.num_rows() == 0 {
+ let data_type = self.data_type(&batch.schema())?;
+ return Ok(ColumnarValue::Array(new_empty_array(&data_type)));
Review Comment:
[P2] Preserve per-row `IN` short-circuiting when making CASE candidates
dynamic. With ANSI enabled and both rows in one native batch, `SELECT id, id IN
(0L, CASE WHEN id = 0 THEN CAST('bad' AS BIGINT) WHEN id = 2 THEN id END) FROM
range(0, 2, 1, 1)` should return `(0,true), (1,NULL)`. This empty-array return
routes the candidate into DataFusion's dynamic `IN`, which evaluates later
candidates against the entire batch unless every row already matched. It
therefore executes the invalid cast for `id=0`, despite that row having matched
`0L`, and aborts the query. The underlying dynamic loop predates this PR, but
this change exposes it for a CASE shape whose base lazy path returned the
expected result. Please evaluate later candidates only for unmatched rows, or
route affected fallible expressions through Spark, and add this mixed-match
regression.
Evidence: Spark 4.1.3 with `spark.sql.ansi.enabled=true` executed the SQL
and returned `[Row(id=0, matched=True), Row(id=1, matched=None)]`; its
optimized plan retained both CASE branches. A disposable Rust reproduction used
an Int64 batch `[0,1]`, `in_list`, the same two CASE branches, and Comet's ANSI
`Cast` from `'bad'` to Int64. The base-equivalent lazy `CaseExpr` path returned
`[true,NULL]`. Head `CaseWhenExpr` returned `Err(External(CastInvalidValue {
value: "bad", from_type: "STRING", to_type: "BIGINT" }))`. Base source selects
that lazy path because this cast is fallible. DataFusion 55.1.0
`InListExpr::evaluate` calls each remaining candidate on the original batch and
only short-circuits when the entire accumulated result is true. Reproduction
source was retained at `/tmp/comet-review-6763-repro.rs`; no project changes
remain.
--
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]
