fornwall opened a new pull request, #24534:
URL: https://github.com/apache/datafusion/pull/24534

   ## Which issue does this PR close?
   
   - Follow-up to apache/datafusion#24516, which fixed apache/datafusion#24513. 
The follow-up was suggested during review of that PR.
   
   ## Rationale for this change
   
   apache/datafusion#24516 conservatively made every scalar subquery nullable 
because an empty result evaluates to `NULL`. Plans guaranteed to return at 
least one row, such as ungrouped aggregates, cannot produce that empty-result 
`NULL`; marking them nullable loses schema precision and prevents valid 
expression simplification.
   
   ## What changes are included in this PR?
   
   - Adds a conservative `LogicalPlan::min_rows()` lower bound.
   - Uses the lower bound in logical schema derivation. Physical 
scalar-subquery planning now defers to `Expr::nullable` instead of hardcoding 
`nullable = true`, so the logical rule is the single source of truth.
   - Makes `max_rows()` account for the one-row form of `EmptyRelation`, 
keeping both bounds consistent.
   
   ## Are these changes tested?
   
   Yes. Unit tests cover row bounds, logical schema derivation, optimizer 
simplification, and physical expression nullability for both possibly empty and 
guaranteed non-empty scalar subqueries. A sqllogictest pins the user-visible 
effect: `EXPLAIN` shows that `(SELECT count(*) FROM empty_table) IS NULL` now 
folds to `false`, eliminating the subquery from the plan. (Query results alone 
cannot show the change: a guaranteed non-empty scalar subquery never evaluates 
to `NULL` at runtime.)
   
   As an ablation check, restoring the conservative always-nullable behavior 
makes the logical, optimizer, physical, and sqllogictest tests fail.
   
   The required all-feature Clippy check and extended workspace test suite pass.
   
   ## Are there any user-facing changes?
   
   Yes. Scalar subqueries proven to return at least one row now preserve the 
nullability of their projected field, allowing valid simplification. 
Potentially empty scalar subqueries remain nullable. `LogicalPlan::min_rows()` 
is a new public API.
   
   ---
   
   AI usage: Implemented with Codex; reviewed and test coverage extended with 
Claude Code. I reviewed the code and made modifications where appropriate.


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