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


##########
datafusion/core/tests/physical_optimizer/limited_distinct_aggregation.rs:
##########
@@ -36,10 +36,17 @@ use 
datafusion_physical_expr_common::sort_expr::PhysicalSortExpr;
 use datafusion_physical_plan::{
     ExecutionPlan,
     aggregates::{AggregateExec, AggregateMode},
-    collect,
+    collect, displayable,
     limit::{GlobalLimitExec, LocalLimitExec},
 };
 
+#[derive(Debug)]
+struct AggregateRuntimeMetric {

Review Comment:
   A few comments here (explaining what the limit and output row fields are in 
particualr) I think would help this test be easier to read



##########
datafusion/physical-plan/src/aggregates/mod.rs:
##########
@@ -3151,6 +3156,103 @@ mod tests {
         Ok(())
     }
 
+    #[tokio::test]
+    async fn limited_distinct_aggregate_uses_migrated_hash_streams() -> 
Result<()> {

Review Comment:
   I would recommend a different term that "migrated" as once the migration is 
complete it will not be relevant anympre
   
   Perhaps something like "limited_distinct_aggregate_uses_partial_hash_stream" 
would be more future proof
   
   



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