2010YOUY01 commented on PR #13897: URL: https://github.com/apache/datafusion/pull/13897#issuecomment-2562108717
> AFAIK the flag is valid in all cases. What do you mean by topK aggregate? > > > Maybe we can mark or comment something like 'experimental' to this method, if it's not tested? > > Let's add a TODO to remind us to add a test exercising this I'm just worried we will miss some corner cases, since whether to maintain input order for AggregateExec is implementation dependent, so I think more tests are necessary. TopK aggregate is just an edge case example, I took a quick look: this path can be triggered if input is ordered, and its output will only guarantee to be ordered by first group key, which not seem correct https://github.com/apache/datafusion/blob/30660e02eb2f1e730d7711685bee46fddfb23c5a/datafusion/physical-plan/src/aggregates/mod.rs#L588-L594 However, since it is not used by the internal optimizer and there is a TODO for testing, it looks good to me -- 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
