Skip to content

#17801 Improve nullability reporting of case expressions #17813

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 13 commits into
base: main
Choose a base branch
from
Draft

#17801 Improve nullability reporting of case expressions #17813

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
408cee1
#17801 Improve nullability reporting of case expressions
pepijnve Sep 28, 2025
045fc9c
#17801 Clarify logical expression test cases
pepijnve Sep 29, 2025
de8b780
#17801 Attempt to clarify const evaluation logic
pepijnve Sep 30, 2025
bbd2949
#17801 Extend predicate const evaluation
pepijnve Sep 30, 2025
2075f4b
#17801 Correctly report nullability of implicit casts in predicates
pepijnve Oct 1, 2025
8c87937
#17801 Code formatting
pepijnve Oct 6, 2025
e155d41
Merge branch 'main' into issue_17801
alamb Oct 8, 2025
5cfe8b6
Merge branch 'main' into issue_17801
alamb Oct 8, 2025
ac4267c
Add comment explaining why the logical plan optimizer is triggered
pepijnve Oct 9, 2025
101db28
Simplify predicate eval code
pepijnve Oct 9, 2025
f4c8579
Code formatting
pepijnve Oct 9, 2025
81b6ec1
Add license header
pepijnve Oct 9, 2025
b6ebd13
Merge branch 'main' into issue_17801
alamb Oct 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion datafusion/core/tests/tpcds_planning.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1052,9 +1052,10 @@ 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?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still dont' get this part. Previously physical plan was created on top of optimized logical, why this step is skipped? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

create_physical_plan calls optimize itself already. Due to the fact that the logical plan was being optimised twice the logical vs physical schema mismatch was being hidden.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To see what I'm talking about, comment out this line on main and run tpcds_physical_q75. You'll get

Error: Internal("Physical input schema should be the same as the one converted from logical input schema. Differences: \n\t- field nullability at index 5 [sales_cnt]: (physical) true vs (logical) false\n\t- field nullability at index 6 [sales_amt]: (physical) true vs (logical) false")

} else {
let _ = state.optimize(&plan)?;
}
}

Expand Down
5 changes: 5 additions & 0 deletions datafusion/expr/src/expr_fn.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,11 @@ pub fn is_null(expr: Expr) -> Expr {
Expr::IsNull(Box::new(expr))
}

/// Create is not null expression
pub fn is_not_null(expr: Expr) -> Expr {
Expr::IsNotNull(Box::new(expr))
}

/// Create is true expression
pub fn is_true(expr: Expr) -> Expr {
Expr::IsTrue(Box::new(expr))
Expand Down
Loading