-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[HZ-707] HOP window function support #20342
Conversation
# Conflicts: # hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/OptimizerContext.java
1bc32c0
to
4094442
Compare
297a247
to
c9603e4
Compare
@@ -42,7 +42,7 @@ | |||
}; | |||
|
|||
public ImposeOrderFunction() { | |||
super("IMPOSE_ORDER", new WindowOperandMetadata(PARAMETERS), RETURN_TYPE_INFERENCE); | |||
super("IMPOSE_ORDER", new WindowOperandMetadata(PARAMETERS, new int[]{2}), RETURN_TYPE_INFERENCE); |
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.
Think the new int[]{2}
bit needs a comment with explanation. Or better yet a constant.
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 argument taking this value is documented...
@Override | ||
public Void visitInputRef(RexInputRef inputRef) { | ||
if (arrayIndexOf(inputRef.getIndex(), indexes) >= 0) { | ||
res[0] = true; |
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.
Any reason this has to be an array? Just checking.
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.
@ivanthescientist outer variable should be effective final to be accessible within the anonymous class/lambda context
Checklist:
Team:
,Type:
,Source:
,Module:
) and Milestone setAdd to Release Notes
orNot Release Notes content
setAlso, fixes #19943