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


##########
datafusion/physical-plan/src/aggregates/aggregate_hash_table/common.rs:
##########
@@ -282,11 +288,48 @@ impl<AggrMode> AggregateHashTable<AggrMode> {
         }
     }
 
+    pub(in crate::aggregates) fn group_by_metrics(&self) -> GroupByMetrics {

Review Comment:
   minor nit does it makes sense to return this as &GroupByMetrics and let the 
caller clone it? It probably doesn't matter



##########
datafusion/sqllogictest/test_files/aggregate_memory_spill.slt:
##########
@@ -58,13 +60,9 @@ FROM (
   GROUP BY (v * 7) % 100000
 )
 ----
-Plan with Metrics
-01)ProjectionExec: expr=[count(Int64(1))@0 as count(*), sum(total)@1 as 
sum(total)], metrics=[<slt:ignore>]
-02)--AggregateExec: mode=Single, gby=[], aggr=[count(Int64(1)), sum(total)], 
metrics=[<slt:ignore>]
-03)----ProjectionExec: expr=[sum(t.v)@1 as total], metrics=[<slt:ignore>]
-04)------AggregateExec: mode=Single, gby=[v@0 * 7 % 100000 as t.v * Int64(7) % 
Int64(100000)], aggr=[sum(t.v)], 
metrics=[<slt:ignore>spill_count=9,<slt:ignore>]
-05)--------ProjectionExec: expr=[value@0 as v], metrics=[<slt:ignore>]
-06)----------LazyMemoryExec: partitions=1, batch_generators=[generate_series: 
start=1, end=100000, batch_size=8192], metrics=[<slt:ignore>]
+<slt:ignore>
+04)------AggregateExec: mode=Single, gby=[v@0 * 7 % 100000 as t.v * Int64(7) % 
Int64(100000)], aggr=[sum(t.v)], 
metrics=[<slt:ignore>spilled_rows=<slt:ignore>K,<slt:ignore>]

Review Comment:
   The changes in aggregate_memory_split.slt seem to have lost the the actual 
value (now just K), but given the comments this seems intentional -- is there a 
reason not to encode the actual row count in the test?
   



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