2010YOUY01 commented on issue #22710:
URL: https://github.com/apache/datafusion/issues/22710#issuecomment-5053722469

   > Hey [@2010YOUY01](https://github.com/2010YOUY01), this refactor looks 
really interesting and I'd like to contribute.
   > 
   > I went through the EPIC and the code, and can see 
[#23657](https://github.com/apache/datafusion/pull/23657) adds spilling for the 
ordered partial/final streams and drops the `MemoryLimit::Finite` bail for 
those two `should_use_*` gates. Since the PR notes that ordered spilling is the 
groundwork the other modes reuse, the unordered paths (like 
should_use_partial_hash_stream, final_hash_stream, single_hash_stream) still 
fall back to `GroupedHashAggregateStream` under a memory limit.
   > 
   > Is this something I can pick up: unordered Partial or Single spilling; 
reusing the ordered larger-than-memory logic from 
[#23657](https://github.com/apache/datafusion/pull/23657) and dropping the 
`Finite` bail? If it collides with your in-flight work, pls point me at 
whatever's most helpful instead, thanks!
   
   Thanks for the interest, for those remaining features, I already have local 
drafts, I hope to complete this EPIC soon.
   
   If you're interested, more test coverage is for aggregation is always 
needed, I think there are still many corners that are not well tested
   - https://github.com/apache/datafusion/issues/23658
   - or look at codecov report, and try to add `sqllogictest`s for the missing 
cases https://app.codecov.io/gh/apache/datafusion


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