RyanSkraba commented on a change in pull request #16856:
URL: https://github.com/apache/beam/pull/16856#discussion_r807730968
##########
File path:
sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/transform/agg/CountIf.java
##########
@@ -31,62 +30,41 @@
public class CountIf {
private CountIf() {}
- public static CountIfFn combineFn() {
- return new CountIf.CountIfFn();
+ public static Combine.CombineFn<Boolean, ?, Long> combineFn() {
+ return new CountIfFn();
}
- public static class CountIfFn extends Combine.CombineFn<Boolean,
CountIfFn.Accum, Long> {
-
- @AutoValue
- public abstract static class Accum implements Serializable {
- abstract boolean isExpressionFalse();
-
- abstract long countIfResult();
-
- static Accum empty() {
- return of(true, 0L);
- }
-
- static Accum of(boolean isExpressionFalse, long countIfResult) {
- return new AutoValue_CountIf_CountIfFn_Accum(isExpressionFalse,
countIfResult);
- }
- }
+ public static class CountIfFn extends Combine.CombineFn<Boolean, long[],
Long> {
Review comment:
Nice cleanup! At this point, would it be worthwhile to let
Count.CountFn be public, so we could just inherit it? I don't really have a
strong opinion.
--
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]
