jayzhan211 commented on code in PR #11943: URL: https://github.com/apache/datafusion/pull/11943#discussion_r1719221322
########## datafusion/expr-common/src/groups_accumulator.rs: ########## @@ -31,6 +31,13 @@ pub enum EmitTo { /// For example, if `n=10`, group_index `0, 1, ... 9` are emitted /// and group indexes '`10, 11, 12, ...` become `0, 1, 2, ...`. First(usize), + /// Emit all groups managed by blocks + AllBlocks, + /// Emit only the first `n` group blocks, + /// similar as `First`, but used in blocked `GroupValues` and `GroupAccumulator`. + /// + /// For example, `n=3`, `block size=4`, finally 12 groups will be returned. + FirstBlocks(usize), Review Comment: > use the iterator approach to impl AllBlocks and FirstBlocks defined now Not sure how does this work, but it looks like a neat idea. If we apply the same idea to "element" (First and All), and consider it as a specialized case with block_size = 1, I think we could end up a pretty nice abstraction. Probably we just need `EmitTo::Block(block_size)` 🤔 However, it is too far way from now. 😆 -- 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