-
Notifications
You must be signed in to change notification settings - Fork 74
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
feat: window function calcite support #172
Merged
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
d37dbdd
refactor: remove unused param on private method
vbarua 88e808d
refactor: pass WindowFunctionConverter through to ExpressionRexConverter
vbarua ee18418
feat: convert Substrait window functions to Calcite RexOvers
vbarua 6ec241c
feat: reject IGNORE NULLS
vbarua 1ae4e86
fix: bad format string
vbarua 9c723ee
refactor: simplify WindowBound
vbarua a62a073
refactor: avoid * imports
vbarua bf91fe4
feat: support for window bounds type
vbarua 6c9947f
refactor: hide visitation behind static methods
vbarua File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,58 +5,62 @@ | |
@Value.Enclosing | ||
public interface WindowBound { | ||
|
||
public BoundedKind boundedKind(); | ||
interface WindowBoundVisitor<R, E extends Throwable> { | ||
R visit(Preceding preceding); | ||
|
||
enum BoundedKind { | ||
UNBOUNDED, | ||
BOUNDED, | ||
CURRENT_ROW | ||
} | ||
R visit(Following following); | ||
|
||
R visit(CurrentRow currentRow); | ||
|
||
enum Direction { | ||
PRECEDING, | ||
FOLLOWING | ||
R visit(Unbounded unbounded); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Direction doesn't have an equivalent in the Substrait spec itself. Calcite has a concept of |
||
|
||
public static CurrentRowWindowBound CURRENT_ROW = | ||
ImmutableWindowBound.CurrentRowWindowBound.builder().build(); | ||
<R, E extends Throwable> R accept(WindowBoundVisitor<R, E> visitor); | ||
|
||
CurrentRow CURRENT_ROW = ImmutableWindowBound.CurrentRow.builder().build(); | ||
Unbounded UNBOUNDED = ImmutableWindowBound.Unbounded.builder().build(); | ||
|
||
@Value.Immutable | ||
abstract static class UnboundedWindowBound implements WindowBound { | ||
@Override | ||
public BoundedKind boundedKind() { | ||
return BoundedKind.UNBOUNDED; | ||
} | ||
abstract class Preceding implements WindowBound { | ||
public abstract long offset(); | ||
|
||
public abstract Direction direction(); | ||
public static Preceding of(long offset) { | ||
return ImmutableWindowBound.Preceding.builder().offset(offset).build(); | ||
} | ||
|
||
public static ImmutableWindowBound.UnboundedWindowBound.Builder builder() { | ||
return ImmutableWindowBound.UnboundedWindowBound.builder(); | ||
@Override | ||
public <R, E extends Throwable> R accept(WindowBoundVisitor<R, E> visitor) { | ||
return visitor.visit(this); | ||
} | ||
} | ||
|
||
@Value.Immutable | ||
abstract static class BoundedWindowBound implements WindowBound { | ||
abstract class Following implements WindowBound { | ||
public abstract long offset(); | ||
|
||
@Override | ||
public BoundedKind boundedKind() { | ||
return BoundedKind.BOUNDED; | ||
public static Following of(long offset) { | ||
return ImmutableWindowBound.Following.builder().offset(offset).build(); | ||
} | ||
|
||
public abstract Direction direction(); | ||
|
||
public abstract long offset(); | ||
@Override | ||
public <R, E extends Throwable> R accept(WindowBoundVisitor<R, E> visitor) { | ||
return visitor.visit(this); | ||
} | ||
} | ||
|
||
public static ImmutableWindowBound.BoundedWindowBound.Builder builder() { | ||
return ImmutableWindowBound.BoundedWindowBound.builder(); | ||
@Value.Immutable | ||
abstract class CurrentRow implements WindowBound { | ||
@Override | ||
public <R, E extends Throwable> R accept(WindowBoundVisitor<R, E> visitor) { | ||
return visitor.visit(this); | ||
} | ||
} | ||
|
||
@Value.Immutable | ||
static class CurrentRowWindowBound implements WindowBound { | ||
abstract class Unbounded implements WindowBound { | ||
@Override | ||
public BoundedKind boundedKind() { | ||
return BoundedKind.CURRENT_ROW; | ||
public <R, E extends Throwable> R accept(WindowBoundVisitor<R, E> visitor) { | ||
return visitor.visit(this); | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Rather than encoding the WindowBound type using a combination of BoundedKind and Direction, I've switched this to 4 classes:
and introduced a WindowBoundVisitor to handle dispatch when converting from these to various other representations.
This representation of Window Bounds: