beyond1920 commented on a change in pull request #15412:
URL: https://github.com/apache/flink/pull/15412#discussion_r603765191
##########
File path:
flink-table/flink-table-planner-blink/src/main/java/org/apache/flink/table/planner/plan/logical/SliceAttachedWindowingStrategy.java
##########
@@ -46,12 +46,13 @@ public SliceAttachedWindowingStrategy(
@JsonProperty(value = FIELD_NAME_TIME_ATTRIBUTE_TYPE) LogicalType
timeAttributeType,
@JsonProperty(FIELD_NAME_SLICE_END) int sliceEnd) {
super(window, timeAttributeType);
+ checkArgument(sliceEnd >= 0);
this.sliceEnd = sliceEnd;
}
@Override
public String toSummaryString(String[] inputFieldNames) {
- checkArgument(sliceEnd >= 0 && sliceEnd < inputFieldNames.length);
+ checkArgument(sliceEnd < inputFieldNames.length);
Review comment:
Could we remove this check, because we already check in constructor?
##########
File path:
flink-table/flink-table-planner-blink/src/main/java/org/apache/flink/table/planner/plan/logical/WindowAttachedWindowingStrategy.java
##########
@@ -58,17 +59,19 @@ public WindowAttachedWindowingStrategy(
public WindowAttachedWindowingStrategy(
WindowSpec window, LogicalType timeAttributeType, int windowEnd) {
super(window, timeAttributeType);
+ checkArgument(windowEnd >= 0);
this.windowStart = -1;
this.windowEnd = windowEnd;
}
@Override
public String toSummaryString(String[] inputFieldNames) {
- checkArgument(windowEnd >= 0 && windowEnd < inputFieldNames.length);
+ checkArgument(windowEnd < inputFieldNames.length);
Review comment:
Could we remove this check, because we already check in constructor?
##########
File path:
flink-table/flink-table-planner-blink/src/main/scala/org/apache/flink/table/planner/plan/nodes/physical/stream/StreamPhysicalLocalWindowAggregate.scala
##########
@@ -60,12 +62,32 @@ class StreamPhysicalLocalWindowAggregate(
windowing.getWindow,
isStateBackendDataViews = false)
+ private lazy val endPropertyName = if
(windowing.isInstanceOf[WindowAttachedWindowingStrategy]) {
+ "window_end"
Review comment:
Could we use match syntax instead of isInstanceOf
--
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:
[email protected]
