jayzhan211 commented on issue #10074: URL: https://github.com/apache/datafusion/issues/10074#issuecomment-2092957671
> I get the sense that part of the problem is that first_value and last_value are some of the most complex aggregate functions Actually, that is the reason why I work on it first, to ensure the design is solid for most of the functions. > What I am worried about is that physical-expr-common would end up with all the code from physical-expr I also thought about this, but I don't think there is more we need to move the code into common, because `first value` covered all the cases except grouping accumulator which is not so different from normal accumulator from the perspective of moving code to `physical-expr-common`, both returns `Accumulator`. I now believe that what I had tried to do `Move create_physical_expr to physical-expr-common` is the wrong step, as it moved out unnecessary things, including `Column` physical-expr which is already moved into common. I agree that the design deserves more discussion. I will work on `covariance` first. -- 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
