[GitHub] [beam] kennknowles commented on a change in pull request #14120: [BEAM-11887] Change SortingFlinkCombineRunner usage to only on Sessions.

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

kennknowles commented on a change in pull request #14120:
URL: https://github.com/apache/beam/pull/14120#discussion_r588947591



##########
File path: runners/flink/src/main/java/org/apache/beam/runners/flink/translation/functions/FlinkMergingNonShuffleReduceFunction.java
##########
@@ -83,7 +83,7 @@ public void reduce(
         new FlinkSideInputReader(sideInputs, getRuntimeContext());
 
     AbstractFlinkCombineRunner<K, InputT, AccumT, OutputT, W> reduceRunner;
-    if (windowingStrategy.getWindowFn().windowCoder().equals(IntervalWindow.getCoder())) {
+    if (windowingStrategy.getWindowFn() instanceof Sessions) {

Review comment:
       I think you want to keep `needsMerge` also. According to the dev list discussion, I changed it so that if the `windowFn` is sessions and `needsMerge` is false, you do the non-sorting (non-merging) group by key and window.




----------------------------------------------------------------
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:
users@infra.apache.org