[GitHub] [flink] JingsongLi commented on issue #8102: [FLINK-12087][table-runtime-blink] Introduce over window operators to blink batch

2019-04-08 Thread GitBox
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

2019-04-08 Thread GitBox
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

2019-04-08 Thread GitBox
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

2019-04-07 Thread GitBox
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

2019-04-07 Thread GitBox
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

2019-04-07 Thread GitBox
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

2019-04-04 Thread GitBox
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