alamb commented on code in PR #17813:
URL: https://github.com/apache/datafusion/pull/17813#discussion_r2545750343
##########
datafusion/expr/src/expr_schema.rs:
##########
@@ -282,14 +284,80 @@ impl ExprSchemable for Expr {
Expr::OuterReferenceColumn(field, _) => Ok(field.is_nullable()),
Expr::Literal(value, _) => Ok(value.is_null()),
Expr::Case(case) => {
Review Comment:
While re-reading this I can't help but think the logic is quite non trivial
- and someone trying to figure out if an expression is nullable on a deeply
nested function might end up calling this function many times
Not for this PR, but I think we should consider how to cache or otherwise
avoid re-computing the same nullabilty (and DataType) expressions over and over
again.
I'll writeup a follow on ticket
##########
datafusion/core/tests/tpcds_planning.rs:
##########
@@ -1052,9 +1052,12 @@ async fn regression_test(query_no: u8, create_physical:
bool) -> Result<()> {
for sql in &sql {
let df = ctx.sql(sql).await?;
let (state, plan) = df.into_parts();
- let plan = state.optimize(&plan)?;
if create_physical {
let _ = state.create_physical_plan(&plan).await?;
Review Comment:
You can see that this test fails with a similar change, outside the context
of this PR, in
- https://github.com/apache/datafusion/pull/18536
--
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]
