RyanSkraba commented on a change in pull request #16856:
URL: https://github.com/apache/beam/pull/16856#discussion_r806931622



##########
File path: 
sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/transform/agg/CountIf.java
##########
@@ -33,21 +37,30 @@ public static CountIfFn combineFn() {
 
   public static class CountIfFn extends Combine.CombineFn<Boolean, 
CountIfFn.Accum, Long> {
 
-    public static class Accum implements Serializable {
-      boolean isExpressionFalse = true;
-      long countIfResult = 0L;
+    @AutoValue
+    public abstract static class Accum implements Serializable {
+      abstract boolean isExpressionFalse();

Review comment:
       Hello!  If I'm reading this correctly and Accum is not used for other 
reasons outside of the class, this is strictly tied to whether countIfResult is 
zero... This might be an opportunity to simplify the code!
   




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


Reply via email to