wudidapaopao commented on code in PR #25990:
URL: https://github.com/apache/datafusion/pull/25990#discussion_r4406277501


##########
datafusion/functions-aggregate/src/count.rs:
##########
@@ -470,6 +483,20 @@ impl AggregateUDFImpl for Count {
     }
 }
 
+fn can_rewrite_count_args(args: &[Expr], info: &SimplifyContext) -> bool {
+    let mut has_non_nullable_column = false;
+    for arg in args {
+        match arg {
+            Expr::Column(_) if matches!(info.nullable(arg), Ok(false)) => {

Review Comment:
   Thanks, you're right. `Dictionary` columns don't get rewritten anymore, and 
I did the same for `RunEndEncoded`, `Union` and `Null` since they can hit the 
same issue. Also added the `register_batch` tests for global and grouped COUNT. 
The global one has a filter so it doesn't get answered from stats.



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