-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Convert Min
and Max
to UDAF
#10943
Comments
I am working on this :) |
Well I am not anymore. We have a problem :D @jayzhan211 the problem is that if we modified the proto and remove min, we remove the 0 value, and this is not allowed in protobuf see https://protobuf.dev/programming-guides/proto3/#enum |
We can change it to 'unknown', as I know Datafusion does not ensure the backward compatibility for proto, so it is fine. We will even remove whole proto after converting all the functions to UDAF |
@jayzhan211 https://github.com/apache/datafusion/pull/11013/files in this draft I started to face a challenge, many tests in expr depends on min/max, and I need to remove them to avoid a circular dependency. Shall we add them back as sql tests? |
For tests in datafusion_expr, you can use function_stub. See |
Is your feature request related to a problem or challenge?
Similar to other issues in #8708
I think the change of this might be large, could split it in several PR.
Describe the solution you'd like
No response
Describe alternatives you've considered
No response
Additional context
No response
The text was updated successfully, but these errors were encountered: