Hasnaathussain commented on code in PR #24484:
URL: https://github.com/apache/datafusion/pull/24484#discussion_r3889640560


##########
datafusion/physical-expr/src/expressions/case.rs:
##########
@@ -141,6 +142,11 @@ impl CaseBody {
                     expr.downcast_ref::<LambdaVariable>()
                 {
                     used_column_indices.insert(lambda_variable.index());
+                } else if expr.downcast_ref::<Literal>().is_none()

Review Comment:
   Addressed in `48b72e44c`: known-safe `ScalarFunctionExpr` leaves remain 
projectable, while unknown leaves use the full-batch path. The regression suite 
also covers a nullary `random()` expression.



##########
datafusion/physical-expr/src/expressions/case.rs:
##########
@@ -1878,6 +1935,38 @@ mod tests {
         Ok(())
     }
 
+    #[test]
+    fn case_without_expr_with_custom_column() -> Result<()> {

Review Comment:
   Added coverage for the three guarded evaluation shapes: searched CASE, 
base-expression CASE, and the single-WHEN-with-ELSE path, each using a 
column-like custom expression.



##########
datafusion/physical-expr/src/expressions/case.rs:
##########
@@ -251,6 +258,7 @@ impl CaseBody {
 struct ProjectedCaseBody {
     projection: Vec<usize>,
     body: CaseBody,
+    supports_projection: bool,

Review Comment:
   Documented the `supports_projection` invariant next to the representation 
and in the `ProjectedCaseBody` docs: when it is false, evaluation must use the 
original CASE body against the full batch.



##########
datafusion/physical-expr/src/expressions/case.rs:
##########
@@ -141,6 +142,11 @@ impl CaseBody {
                     expr.downcast_ref::<LambdaVariable>()
                 {
                     used_column_indices.insert(lambda_variable.index());
+                } else if expr.downcast_ref::<Literal>().is_none()
+                    && expr.children().is_empty()
+                {
+                    // Unknown leaves may read input columns without exposing 
a Column child.

Review Comment:
   The PR description now calls this `Part of #21231` and states the scope 
explicitly. The dependency-reporting extension discussed in the issue remains 
separate work.



-- 
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]

Reply via email to