Repository navigation
Support Framing for Window Aggregations - #13514
Conversation
702e413 to
149a4c4
Compare
149a4c4 to
f27a86d
Compare
|
This pull request fixes 1 alert when merging f27a86d into 83261f9 - view on LGTM.com fixed alerts:
Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog. |
|
This pull request fixes 1 alert when merging 9870923 into 9679f6a - view on LGTM.com fixed alerts:
Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog. |
|
This pull request fixes 1 alert when merging 7f38000 into b56855b - view on LGTM.com fixed alerts:
Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog. |
|
This pull request fixes 1 alert when merging c5f32b0 into b56855b - view on LGTM.com fixed alerts:
Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog. |
This adds support for framing over ROWS for window aggregations. Still not implemented as yet: 1. RANGE frames 2. Multiple different frames in the same query 3. Frames on last/first functions
c5f32b0 to
4e1660e
Compare
| public class WindowFrame | ||
| { | ||
| @SuppressWarnings("unused") | ||
| public enum PeerType |
There was a problem hiding this comment.
I think this is worth a javadoc, it is described in some other places, such as FramedOnHeapAggregatable, but here would be nice too probably, is fine to defer until later when we have more support for this stuff though too
There was a problem hiding this comment.
Yeah, word. This is a SQL concept stolen from... SQL, so I didn't think to document it as much as I probably should have. When we add support for RANGE peers, we can remove the suppression and actually document it.
|
|
||
| import java.util.Objects; | ||
|
|
||
| public class WindowFrame |
There was a problem hiding this comment.
heh, its a shame that "frame" really is the best name this; hopefully it wont be too confusing since it doesn't really have anything to do with the other one, https://github.com/apache/druid/blob/master/processing/src/main/java/org/apache/druid/frame/Frame.java which should maybe be called DataFrame or... idk naming is just the worst 😅
There was a problem hiding this comment.
DataFrame is a thing in R too... I don't think it's far from what a org.apache.druid.frame.Frame is though, so maybe that's actually a good name?
| // In this case, there are not enough rows to completely build up the full window aperture before it needs to | ||
| // also start contracting the aperture because of the upper offset. So we use a method that specifically | ||
| // handles checks for both expanding and reducing the aperture on every iteration. |
There was a problem hiding this comment.
just wanted to say thanks for all the comments in this class, a lot going on here and it helps out 👍
There was a problem hiding this comment.
little secret: I needed them for myself :P
some code is "self documenting" some code is not, this code is not.
* Support Framing for Window Aggregations This adds support for framing over ROWS for window aggregations. Still not implemented as yet: 1. RANGE frames 2. Multiple different frames in the same query 3. Frames on last/first functions
3 participants
This adds support for framing over ROWS for window aggregations.
Still not implemented as yet:
This PR also brings with it some adjustments and improvements to the tests for semantic interfaces and RowsAndColumns implementations.
The PR touches a lot of files, that's mostly because I combined all of the "semantic interfaces" into a
semanticpackage and that affected a bunch of imports.The PR also looks like it's a lot of lines of code, but most of that is actually whitespace changes on some of the test files done by auto-formatting. Checking them in now so that future auto-formatting is safe from generating whitespace changes.
The primary code to review is essentially all inside of FramedOnHeapAggregatable, the tests associated with it and some in the planning.
Lastly, there are some code cleanup/renames being included that were asked for in the previous PR. (Things like completing the renaming of
celland removing the TODOs)This PR has: