Miscellaneous ArrayData Cleanup #5612
Conversation
github-actions
bot
added
core
| let rows = self | ||
| .aggregate( | ||
| vec![], | ||
| vec![datafusion_expr::count(Expr::Literal(ScalarValue::Null))], |
There was a problem hiding this comment.
This was working by accident, as the null count for NullArray was computed incorrectly
datafusion/core/src/dataframe.rs
Outdated
| "Unexpected output when collecting for count()".to_string(), | ||
| ))? as usize; | ||
| Ok(len) | ||
| let mut stream = self.execute_stream().await?; |
There was a problem hiding this comment.
I'm not sure if there is a better way to express this, in particular I'm not sure what select count(*) from table would compile down to, I can't help feeling there should be a better way to express this for the optimiser
There was a problem hiding this comment.
You could keep the old behaviour and use datafusion_exr::utils::COUNT_STAR_EXPANSION instead of the ScalarValue::Null, since count(*) is expanded to that in SQL planner (related bug #5518 prevents directly using Count(Expr:Wildcard) at the moment)
The reason I had it that way was to let the count optimize out any column projections to only the minimal needed, which I think theoretically is faster than just executing the plan as is and counting the rows
Which issue does this PR close?
Closes #.
Rationale for this change
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?