alamb commented on code in PR #11261:
URL: https://github.com/apache/datafusion/pull/11261#discussion_r1675789494


##########
datafusion/core/src/physical_optimizer/aggregate_statistics.rs:
##########
@@ -273,6 +263,44 @@ fn take_optimizable_max(
     None
 }
 
+fn is_non_distinct_count(agg_expr: &dyn AggregateExpr) -> bool {
+    if let Some(agg_expr) = 
agg_expr.as_any().downcast_ref::<AggregateFunctionExpr>() {
+        if agg_expr.fun().name() == "count" && !agg_expr.is_distinct() {
+            return true;
+        }
+    }
+
+    false
+}
+
+fn is_min(agg_expr: &dyn AggregateExpr) -> bool {

Review Comment:
   ```suggestion
   // TODO: Move this check into AggregateUDFImpl
   // https://github.com/apache/datafusion/issues/11153
   fn is_min(agg_expr: &dyn AggregateExpr) -> bool {
   ```



##########
datafusion/core/src/physical_optimizer/aggregate_statistics.rs:
##########
@@ -273,6 +263,44 @@ fn take_optimizable_max(
     None
 }
 
+fn is_non_distinct_count(agg_expr: &dyn AggregateExpr) -> bool {

Review Comment:
   I really like the encapsulation of this logic into functions 👍 
   
   ```suggestion
   // TODO: Move this check into AggregateUDFImpl
   // https://github.com/apache/datafusion/issues/11153
   fn is_non_distinct_count(agg_expr: &dyn AggregateExpr) -> bool {
   ```



##########
datafusion/core/src/physical_optimizer/aggregate_statistics.rs:
##########
@@ -273,6 +263,44 @@ fn take_optimizable_max(
     None
 }
 
+fn is_non_distinct_count(agg_expr: &dyn AggregateExpr) -> bool {
+    if let Some(agg_expr) = 
agg_expr.as_any().downcast_ref::<AggregateFunctionExpr>() {
+        if agg_expr.fun().name() == "count" && !agg_expr.is_distinct() {
+            return true;
+        }
+    }
+
+    false
+}
+
+fn is_min(agg_expr: &dyn AggregateExpr) -> bool {
+    if agg_expr.as_any().is::<expressions::Min>() {
+        return true;
+    }
+
+    if let Some(agg_expr) = 
agg_expr.as_any().downcast_ref::<AggregateFunctionExpr>() {
+        if agg_expr.fun().name() == "min" {
+            return true;
+        }
+    }
+
+    false
+}
+
+fn is_max(agg_expr: &dyn AggregateExpr) -> bool {

Review Comment:
   ```suggestion
   // TODO: Move this check into AggregateUDFImpl
   // https://github.com/apache/datafusion/issues/11153
   fn is_max(agg_expr: &dyn AggregateExpr) -> bool {
   ```



-- 
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: github-unsubscr...@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to