Repository navigation
Convert Min and Max to UDAF #10943
Description
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
Activity
edmondop commentedon Jun 18, 2024
I am working on this :)
edmondop commentedon Jun 18, 2024
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
https://github.com/apache/datafusion/pull/10898/files#diff-dfbd8463b9c4a1e7ac21cf0154a33966bb72a86df15d5e1409906a08cbdeb4dfR475
jayzhan211 commentedon Jun 19, 2024
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
edmondop commentedon Jun 19, 2024
@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?
jayzhan211 commentedon Jun 19, 2024
For tests in datafusion_expr, you can use function_stub. See
datafusion/expr/src/test/function_stub.rsFor tests in optimizer or others, you can import function in dev-dependencies or move them to slt.