-
Notifications
You must be signed in to change notification settings - Fork 3.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
Upgrade to Pinot 0.12.1 #16623
Upgrade to Pinot 0.12.1 #16623
Conversation
c111772
to
bc53c7a
Compare
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 documentation says we support "Pinot 0.10.0 or higher". Is that still true after this change?
for (TransformFunctionType transformFunctionType : TransformFunctionType.values()) { | ||
for (String alias : transformFunctionType.getAliases()) { | ||
TransformFunctionType previousValue = builder.put(canonicalize(alias), transformFunctionType); | ||
checkState(previousValue == null || previousValue == transformFunctionType, "Duplicate key with different values for alias '%s', transform function type '%s' and previous value '%s'", canonicalize(alias), transformFunctionType, previousValue); |
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.
Did you mean to use ==
for comparing TransformFunctionType
? Does it not implement equality and we need to use identity?
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.
It's an enum, is that ok?
...rino-pinot/src/main/java/io/trino/plugin/pinot/query/PinotTransformFunctionTypeResolver.java
Outdated
Show resolved
Hide resolved
plugin/trino-pinot/src/main/java/io/trino/plugin/pinot/query/PinotExpressionRewriter.java
Outdated
Show resolved
Hide resolved
.../trino-pinot/src/test/java/io/trino/plugin/pinot/BasePinotIntegrationConnectorSmokeTest.java
Show resolved
Hide resolved
0cd07a5
to
86d2a88
Compare
Updated, thanks for catching that! |
...rino-pinot/src/main/java/io/trino/plugin/pinot/query/PinotTransformFunctionTypeResolver.java
Outdated
Show resolved
Hide resolved
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
Upgrade pinot libraries to the latest version.
Release notes
(x) This is not user-visible or docs only and no release notes are required.