-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Enable bytes and string data types for max/min udaf #9081
Enable bytes and string data types for max/min udaf #9081
Conversation
- The max and min udaf was generalized to handle date & time data types See - 5134393 - As a result, the code was capable of supporting string and bytes data types - This commit simply enables them in the max and min udaf's
@@ -52,6 +52,8 @@ public KsqlAggregateFunction createAggregateFunction( | |||
case DATE: | |||
case TIME: | |||
case TIMESTAMP: | |||
case STRING: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch that STRINGs weren't supported or mentioned on the issue: #8913
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code already had support for those types. @jzaralim also asked if we could support strings along with bytes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Description
Enable bytes n string data types for max/min udaf
Testing done
Added unit and integration tests
Reviewer checklist