[GitHub] [flink] JingsongLi commented on issue #8102: [FLINK-12087][table-runtime-blink] Introduce over window operators to blink batch
JingsongLi commented on issue #8102: [FLINK-12087][table-runtime-blink] Introduce over window operators to blink batch URL: https://github.com/apache/flink/pull/8102#issuecomment-480731189 I try implement it like "immediately determine the left and right boundary", but even if we immediately determine the boundary, we need take the record one by one for calculation, so the final implementation is basically the same as the current one. You can give me more details about a better understanding implement. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [flink] JingsongLi commented on issue #8102: [FLINK-12087][table-runtime-blink] Introduce over window operators to blink batch
JingsongLi commented on issue #8102: [FLINK-12087][table-runtime-blink] Introduce over window operators to blink batch URL: https://github.com/apache/flink/pull/8102#issuecomment-480702530 >> As I said before, you can immediately determine the left and right boundary with currentIndex - precedingCount, currentIndex + followingCount I think we don't need this optimization now. In `RowSlidingFrame`, we don't need immediately determine boundary. Because it must be head boundary plus 1 and end boundary plus 1, Even if change to immediately determine boundary. The algorithm complexity is the same. See more detail in `SlidingOverFrame.process`, next current row computation will reuse previous buffer. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [flink] JingsongLi commented on issue #8102: [FLINK-12087][table-runtime-blink] Introduce over window operators to blink batch
JingsongLi commented on issue #8102: [FLINK-12087][table-runtime-blink] Introduce over window operators to blink batch URL: https://github.com/apache/flink/pull/8102#issuecomment-480694448 Consider a new `BaseSlidingOverFrame`: It has two abstract method: 1.`public abstract boolean isLeftBoundRowInWindow(BaseRow inputRow, int inputIndex, BaseRow currentRow, int currentIndex);` 2.`public abstract boolean isRightBoundInWindow(BaseRow inputRow, int inputIndex, BaseRow currentRow, int currentIndex);` In `process`, it will invoke `isLeftBoundRowInWindow` and `isRightBoundInWindow` to determine boundaries. In `RowSlidingOverFrame`: ``` @Override public boolean isLeftBoundRowInWindow(BaseRow inputRow, int inputIndex, BaseRow currentRow, int currentIndex) { return inputIndex >= currentIndex + leftOffset; } @Override public boolean isRightBoundInWindow(BaseRow inputRow, int inputIndex, BaseRow currentRow, int currentIndex) { return inputIndex <= currentIndex + rightOffset; } ``` In `RangeSlidingOverFrame`: ``` @Override public boolean isLeftBoundRowInWindow(BaseRow inputRow, int inputIndex, BaseRow currentRow, int currentIndex) { return lbound.compare(inputRow, currentRow) >= 0; } @Override public boolean isRightBoundInWindow(BaseRow inputRow, int inputIndex, BaseRow currentRow, int currentIndex) { return rbound.compare(inputRow, currentRow) <= 0; } ``` I mean the `isLeftBoundRowInWindow` and `isRightBoundInWindow` make thing more complicated. Still has args: `(BaseRow inputRow, int inputIndex, BaseRow currentRow, int currentIndex)`. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [flink] JingsongLi commented on issue #8102: [FLINK-12087][table-runtime-blink] Introduce over window operators to blink batch
JingsongLi commented on issue #8102: [FLINK-12087][table-runtime-blink] Introduce over window operators to blink batch URL: https://github.com/apache/flink/pull/8102#issuecomment-480671968 > Why `RowSlidingOverFrame` needs a comparator class? Don't need a comparator class, but need a compare method to reuse code in `BaseSlidingOverFrame`. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [flink] JingsongLi commented on issue #8102: [FLINK-12087][table-runtime-blink] Introduce over window operators to blink batch
JingsongLi commented on issue #8102: [FLINK-12087][table-runtime-blink] Introduce over window operators to blink batch URL: https://github.com/apache/flink/pull/8102#issuecomment-480671767 > Why `RowSlidingOverFrame` needs a comparator class? consider sql: `order by c range Unbounded PRECEDING and CURRENT ROW` The end bound of this frame is range current row, that mean `inputRow` less than or equal to `currentRow` should be included in the window. So the comparator do this comparison. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [flink] JingsongLi commented on issue #8102: [FLINK-12087][table-runtime-blink] Introduce over window operators to blink batch
JingsongLi commented on issue #8102: [FLINK-12087][table-runtime-blink] Introduce over window operators to blink batch URL: https://github.com/apache/flink/pull/8102#issuecomment-480660556 @KurtYoung I think that even with `RowSlidingOverFrame` and `RangeSlidingOverFrame`, still need a interface like `compare(BaseRow inputRow, int inputIndex, BaseRow currentRow, int currentIndex)` for code reuse, so I prefer to keep `BoundComparator` and not to introduce `RowXXFrame` and `RangeXXFrame`. I add the default implements `RowBoundComparator` and `RangeBoundComparator` to better understand `BoundComparator`. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [flink] JingsongLi commented on issue #8102: [FLINK-12087][table-runtime-blink] Introduce over window operators to blink batch
JingsongLi commented on issue #8102: [FLINK-12087][table-runtime-blink] Introduce over window operators to blink batch URL: https://github.com/apache/flink/pull/8102#issuecomment-479776062 Thanks @KurtYoung for reviewing, I changed method name and add comments. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services