[GitHub] [flink] beyond1920 commented on a change in pull request #18331: [FLINK-25614][table/runtime] Let LocalWindowAggregate be chained with upstream

[GitBox <gi...@apache.org>]

beyond1920 commented on a change in pull request #18331:
URL: https://github.com/apache/flink/pull/18331#discussion_r792467756



##########
File path: flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/operators/window/slicing/SliceAssigners.java
##########
@@ -401,7 +402,8 @@ public WindowedSliceAssigner(int windowEndIndex, SliceAssigner innerAssigner) {
 
         @Override
         public long assignSliceEnd(RowData element, ClockService clock) {
-            return element.getLong(windowEndIndex);
+            TimestampData windowEnd = element.getTimestamp(windowEndIndex, 3);

Review comment:
       How about 
   `return element.getTimestamp(windowEndIndex, 3).getMillisecond();`

##########
File path: flink-table/flink-table-runtime/src/test/java/org/apache/flink/table/runtime/operators/aggregate/window/SlicingWindowAggOperatorTest.java
##########
@@ -963,6 +965,10 @@ private static long epochMills(ZoneId shiftTimeZone, String timestampStr) {
         return localDateTime.toInstant(zoneOffset).toEpochMilli();
     }
 
+    private static TimestampData wrapTs(long epochMillis) {

Review comment:
       How about import static  `TimestampData.fromEpochMillis` and use `fromEpochMillis` directly instead of add an extra `wrapTs` method?




-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org