gemini-code-assist[bot] commented on code in PR #37715:
URL: https://github.com/apache/beam/pull/37715#discussion_r2867124372
##########
runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/DataflowPipelineTranslator.java:
##########
@@ -970,8 +1051,8 @@ private <K, V> void groupByKeyHelper(
&& windowingStrategy.getWindowFn().assignsToOneWindow();
if (isStreaming) {
allowCombinerLifting &= transform.fewKeys();
- // TODO: Allow combiner lifting on the non-default trigger, as
appropriate.
- allowCombinerLifting &= (windowingStrategy.getTrigger()
instanceof DefaultTrigger);
+ allowCombinerLifting &=
+ windowingStrategy.getTrigger().accept(new
TriggerCombinerLiftingCompatibility());
Review Comment:
Since `TriggerCombinerLiftingCompatibility` is stateless, it can be
instantiated once as a `static final` field to avoid creating a new object on
each call. This is a minor optimization but good practice.
For example, you can add the field near the class definition:
```java
private static class TriggerCombinerLiftingCompatibility implements
TriggerVisitor<Boolean> {
// ... methods
}
private static final TriggerCombinerLiftingCompatibility
COMBINER_LIFTING_COMPATIBILITY_VISITOR =
new TriggerCombinerLiftingCompatibility();
```
And then use the constant here.
```suggestion
allowCombinerLifting &=
windowingStrategy.getTrigger().accept(COMBINER_LIFTING_COMPATIBILITY_VISITOR);
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
