You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "xiangfu0 (via GitHub)" <gi...@apache.org> on 2024/05/26 06:01:54 UTC

[PR] Add FunnelCompleteCount function [pinot]

xiangfu0 opened a new pull request, #13231:
URL: https://github.com/apache/pinot/pull/13231

   - Add `FunnelCompleteCountAggregationFunction` to calculate how many completed count of funnels
   - Extract common logic to `FunnelBaseAggregationFunction` for both `FunnelMaxStepAggregationFunction` and `FunnelCompleteCountAggregationFunction`


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Add FunnelCompleteCount aggregation function [pinot]

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #13231:
URL: https://github.com/apache/pinot/pull/13231#discussion_r1623632026


##########
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/ArrayFunctions.java:
##########
@@ -230,6 +230,42 @@ public static String arrayElementAtString(String[] arr, int idx) {
     return idx > 0 && idx <= arr.length ? arr[idx - 1] : NullValuePlaceHolder.STRING;
   }
 
+  @ScalarFunction
+  public static int arraySumInt(int[] arr) {
+    int sum = 0;
+    for (int value : arr) {
+      sum += value;
+    }
+    return sum;
+  }
+
+  @ScalarFunction
+  public static long arraySumLong(long[] arr) {
+    long sum = 0;
+    for (long value : arr) {
+      sum += value;
+    }
+    return sum;
+  }
+
+  @ScalarFunction
+  public static double arraySumFloat(float[] arr) {

Review Comment:
   ```suggestion
     public static float arraySumFloat(float[] arr) {
   ```



##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/funnel/window/FunnelBaseAggregationFunction.java:
##########
@@ -32,33 +32,34 @@
 import org.apache.pinot.core.query.aggregation.AggregationResultHolder;
 import org.apache.pinot.core.query.aggregation.ObjectAggregationResultHolder;
 import org.apache.pinot.core.query.aggregation.function.AggregationFunction;
+import org.apache.pinot.core.query.aggregation.function.funnel.FunnelStepEvent;
 import org.apache.pinot.core.query.aggregation.groupby.GroupByResultHolder;
 import org.apache.pinot.core.query.aggregation.groupby.ObjectGroupByResultHolder;
-import org.apache.pinot.segment.spi.AggregationFunctionType;
 
 
-public class FunnelMaxStepAggregationFunction implements AggregationFunction<PriorityQueue<FunnelStepEvent>, Long> {
-  private final ExpressionContext _timestampExpression;
-  private final long _windowSize;
-  private final List<ExpressionContext> _stepExpressions;
-  private final FunnelModes _modes = new FunnelModes();
-  private final int _numSteps;
+public abstract class FunnelBaseAggregationFunction<F extends Comparable>
+    implements AggregationFunction<PriorityQueue<FunnelStepEvent>, F> {
+  protected final ExpressionContext _timestampExpression;
+  protected final long _windowSize;
+  protected final List<ExpressionContext> _stepExpressions;
+  protected final FunnelModes _modes = new FunnelModes();
+  protected final int _numSteps;
 
-  public FunnelMaxStepAggregationFunction(List<ExpressionContext> arguments) {
+  public FunnelBaseAggregationFunction(List<ExpressionContext> arguments) {
     int numArguments = arguments.size();
     Preconditions.checkArgument(numArguments > 2,
-        "FUNNELMAXSTEP expects >= 3 arguments, got: %s. The function can be used as "
+        "FUNNEL_BASE_FUNC expects >= 3 arguments, got: %s. The function can be used as "
             + "funnelMaxStep(timestampExpression, windowSize, ARRAY[stepExpression, ..], [mode, [mode, ... ]])",
         numArguments);
     _timestampExpression = arguments.get(0);
     _windowSize = arguments.get(1).getLiteral().getLongValue();
     Preconditions.checkArgument(_windowSize > 0, "Window size must be > 0");
     ExpressionContext stepExpressionContext = arguments.get(2);
     if (stepExpressionContext.getFunction() != null) {
-      // LEAF stage init this function like funnelmaxstep($1,'1000',arrayValueConstructor($2,$3,$4,...))
+      // LEAF stage init this function like funnelBaseFunc($1,'1000',arrayValueConstructor($2,$3,$4,...))
       _stepExpressions = stepExpressionContext.getFunction().getArguments();
     } else {
-      // Intermediate Stage init this function like funnelmaxstep($1,'1000',__PLACEHOLDER__)
+      // Intermediate Stage init this function like funnelBaseFunc($1,'1000',__PLACEHOLDER__)

Review Comment:
   In the test, if you add the aggregate hint `is_skip_leaf_stage_group_by` to push the aggregate to the intermediate stage, I believe it won't be able to find the function properly



##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/funnel/window/FunnelBaseAggregationFunction.java:
##########
@@ -32,33 +32,34 @@
 import org.apache.pinot.core.query.aggregation.AggregationResultHolder;
 import org.apache.pinot.core.query.aggregation.ObjectAggregationResultHolder;
 import org.apache.pinot.core.query.aggregation.function.AggregationFunction;
+import org.apache.pinot.core.query.aggregation.function.funnel.FunnelStepEvent;
 import org.apache.pinot.core.query.aggregation.groupby.GroupByResultHolder;
 import org.apache.pinot.core.query.aggregation.groupby.ObjectGroupByResultHolder;
-import org.apache.pinot.segment.spi.AggregationFunctionType;
 
 
-public class FunnelMaxStepAggregationFunction implements AggregationFunction<PriorityQueue<FunnelStepEvent>, Long> {
-  private final ExpressionContext _timestampExpression;
-  private final long _windowSize;
-  private final List<ExpressionContext> _stepExpressions;
-  private final FunnelModes _modes = new FunnelModes();
-  private final int _numSteps;
+public abstract class FunnelBaseAggregationFunction<F extends Comparable>
+    implements AggregationFunction<PriorityQueue<FunnelStepEvent>, F> {
+  protected final ExpressionContext _timestampExpression;
+  protected final long _windowSize;
+  protected final List<ExpressionContext> _stepExpressions;
+  protected final FunnelModes _modes = new FunnelModes();
+  protected final int _numSteps;
 
-  public FunnelMaxStepAggregationFunction(List<ExpressionContext> arguments) {
+  public FunnelBaseAggregationFunction(List<ExpressionContext> arguments) {
     int numArguments = arguments.size();
     Preconditions.checkArgument(numArguments > 2,
-        "FUNNELMAXSTEP expects >= 3 arguments, got: %s. The function can be used as "
+        "FUNNEL_BASE_FUNC expects >= 3 arguments, got: %s. The function can be used as "
             + "funnelMaxStep(timestampExpression, windowSize, ARRAY[stepExpression, ..], [mode, [mode, ... ]])",
         numArguments);
     _timestampExpression = arguments.get(0);
     _windowSize = arguments.get(1).getLiteral().getLongValue();
     Preconditions.checkArgument(_windowSize > 0, "Window size must be > 0");
     ExpressionContext stepExpressionContext = arguments.get(2);
     if (stepExpressionContext.getFunction() != null) {
-      // LEAF stage init this function like funnelmaxstep($1,'1000',arrayValueConstructor($2,$3,$4,...))
+      // LEAF stage init this function like funnelBaseFunc($1,'1000',arrayValueConstructor($2,$3,$4,...))
       _stepExpressions = stepExpressionContext.getFunction().getArguments();
     } else {
-      // Intermediate Stage init this function like funnelmaxstep($1,'1000',__PLACEHOLDER__)
+      // Intermediate Stage init this function like funnelBaseFunc($1,'1000',__PLACEHOLDER__)

Review Comment:
   Since we already know intermediate stage won't be able to fill function properly, should we change it to not relying on parsing the function? We can pass in `numSteps` before each individual step expressions.
   At high level, relying on function to be passed into aggregate is anti-pattern because function will be extracted out in `PROJECT` as a reference



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Add FunnelCompleteCount aggregation function [pinot]

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #13231:
URL: https://github.com/apache/pinot/pull/13231#discussion_r1624894425


##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/funnel/window/FunnelBaseAggregationFunction.java:
##########
@@ -32,45 +31,35 @@
 import org.apache.pinot.core.query.aggregation.AggregationResultHolder;
 import org.apache.pinot.core.query.aggregation.ObjectAggregationResultHolder;
 import org.apache.pinot.core.query.aggregation.function.AggregationFunction;
+import org.apache.pinot.core.query.aggregation.function.funnel.FunnelStepEvent;
 import org.apache.pinot.core.query.aggregation.groupby.GroupByResultHolder;
 import org.apache.pinot.core.query.aggregation.groupby.ObjectGroupByResultHolder;
-import org.apache.pinot.segment.spi.AggregationFunctionType;
 
 
-public class FunnelMaxStepAggregationFunction implements AggregationFunction<PriorityQueue<FunnelStepEvent>, Long> {
-  private final ExpressionContext _timestampExpression;
-  private final long _windowSize;
-  private final List<ExpressionContext> _stepExpressions;
-  private final FunnelModes _modes = new FunnelModes();
-  private final int _numSteps;
+public abstract class FunnelBaseAggregationFunction<F extends Comparable>
+    implements AggregationFunction<PriorityQueue<FunnelStepEvent>, F> {
+  protected final ExpressionContext _timestampExpression;
+  protected final long _windowSize;
+  protected final List<ExpressionContext> _stepExpressions;
+  protected final FunnelModes _modes = new FunnelModes();
+  protected final int _numSteps;
 
-  public FunnelMaxStepAggregationFunction(List<ExpressionContext> arguments) {
+  public FunnelBaseAggregationFunction(List<ExpressionContext> arguments) {
     int numArguments = arguments.size();
     Preconditions.checkArgument(numArguments > 2,

Review Comment:
   ```suggestion
       Preconditions.checkArgument(numArguments > 3,
   ```



##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/funnel/window/FunnelBaseAggregationFunction.java:
##########
@@ -32,45 +31,35 @@
 import org.apache.pinot.core.query.aggregation.AggregationResultHolder;
 import org.apache.pinot.core.query.aggregation.ObjectAggregationResultHolder;
 import org.apache.pinot.core.query.aggregation.function.AggregationFunction;
+import org.apache.pinot.core.query.aggregation.function.funnel.FunnelStepEvent;
 import org.apache.pinot.core.query.aggregation.groupby.GroupByResultHolder;
 import org.apache.pinot.core.query.aggregation.groupby.ObjectGroupByResultHolder;
-import org.apache.pinot.segment.spi.AggregationFunctionType;
 
 
-public class FunnelMaxStepAggregationFunction implements AggregationFunction<PriorityQueue<FunnelStepEvent>, Long> {
-  private final ExpressionContext _timestampExpression;
-  private final long _windowSize;
-  private final List<ExpressionContext> _stepExpressions;
-  private final FunnelModes _modes = new FunnelModes();
-  private final int _numSteps;
+public abstract class FunnelBaseAggregationFunction<F extends Comparable>
+    implements AggregationFunction<PriorityQueue<FunnelStepEvent>, F> {
+  protected final ExpressionContext _timestampExpression;
+  protected final long _windowSize;
+  protected final List<ExpressionContext> _stepExpressions;
+  protected final FunnelModes _modes = new FunnelModes();
+  protected final int _numSteps;
 
-  public FunnelMaxStepAggregationFunction(List<ExpressionContext> arguments) {
+  public FunnelBaseAggregationFunction(List<ExpressionContext> arguments) {
     int numArguments = arguments.size();
     Preconditions.checkArgument(numArguments > 2,
-        "FUNNELMAXSTEP expects >= 3 arguments, got: %s. The function can be used as "
-            + "funnelMaxStep(timestampExpression, windowSize, ARRAY[stepExpression, ..], [mode, [mode, ... ]])",
+        "FUNNEL_AGG_FUNC expects >= 4 arguments, got: %s. The function can be used as "
+            + getType().getName() + "(timestampExpression, windowSize, numberSteps, stepExpression, "
+            + "[stepExpression, ..], [mode, [mode, ... ]])",
         numArguments);
     _timestampExpression = arguments.get(0);
     _windowSize = arguments.get(1).getLiteral().getLongValue();
     Preconditions.checkArgument(_windowSize > 0, "Window size must be > 0");
-    ExpressionContext stepExpressionContext = arguments.get(2);
-    if (stepExpressionContext.getFunction() != null) {
-      // LEAF stage init this function like funnelmaxstep($1,'1000',arrayValueConstructor($2,$3,$4,...))
-      _stepExpressions = stepExpressionContext.getFunction().getArguments();
-    } else {
-      // Intermediate Stage init this function like funnelmaxstep($1,'1000',__PLACEHOLDER__)
-      _stepExpressions = ImmutableList.of();
-    }
-    if (numArguments > 3) {
-      arguments.subList(3, numArguments)
+    _numSteps = arguments.get(2).getLiteral().getIntValue();
+    _stepExpressions = arguments.subList(3, 3 + _numSteps);

Review Comment:
   Add an argument count check?



##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/AggregationFunctionType.java:
##########
@@ -503,4 +510,16 @@ public static AggregationFunctionType getAggregationFunctionType(String function
       }
     }
   }
+
+  static class IntArrayReturnTypeInference implements SqlReturnTypeInference {
+    static final IntArrayReturnTypeInference INSTANCE = new IntArrayReturnTypeInference();
+
+    @Override
+    public @org.checkerframework.checker.nullness.qual.Nullable RelDataType inferReturnType(

Review Comment:
   Remove the annotation. It won't return return `null`.



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Add FunnelCompleteCount aggregation function [pinot]

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #13231:
URL: https://github.com/apache/pinot/pull/13231#issuecomment-2132094640

   ## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/13231?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   Attention: Patch coverage is `0%` with `89 lines` in your changes are missing coverage. Please review.
   > Project coverage is 0.00%. Comparing base [(`59551e4`)](https://app.codecov.io/gh/apache/pinot/commit/59551e45224f1535c4863fd577622b37366ccc97?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`1cb05bc`)](https://app.codecov.io/gh/apache/pinot/commit/1cb05bc7ae7832a65e88672a5a1ee03284a51c65?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   > Report is 499 commits behind head on master.
   
   | [Files](https://app.codecov.io/gh/apache/pinot/pull/13231?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
   |---|---|---|
   | [...window/FunnelCompleteCountAggregationFunction.java](https://app.codecov.io/gh/apache/pinot/pull/13231?src=pr&el=tree&filepath=pinot-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcore%2Fquery%2Faggregation%2Ffunction%2Ffunnel%2Fwindow%2FFunnelCompleteCountAggregationFunction.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9mdW5uZWwvd2luZG93L0Z1bm5lbENvbXBsZXRlQ291bnRBZ2dyZWdhdGlvbkZ1bmN0aW9uLmphdmE=) | 0.00% | [42 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/13231?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...unnel/window/FunnelMaxStepAggregationFunction.java](https://app.codecov.io/gh/apache/pinot/pull/13231?src=pr&el=tree&filepath=pinot-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcore%2Fquery%2Faggregation%2Ffunction%2Ffunnel%2Fwindow%2FFunnelMaxStepAggregationFunction.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9mdW5uZWwvd2luZG93L0Z1bm5lbE1heFN0ZXBBZ2dyZWdhdGlvbkZ1bmN0aW9uLmphdmE=) | 0.00% | [42 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/13231?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...n/funnel/window/FunnelBaseAggregationFunction.java](https://app.codecov.io/gh/apache/pinot/pull/13231?src=pr&el=tree&filepath=pinot-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcore%2Fquery%2Faggregation%2Ffunction%2Ffunnel%2Fwindow%2FFunnelBaseAggregationFunction.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9mdW5uZWwvd2luZG93L0Z1bm5lbEJhc2VBZ2dyZWdhdGlvbkZ1bmN0aW9uLmphdmE=) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/13231?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...che/pinot/segment/spi/AggregationFunctionType.java](https://app.codecov.io/gh/apache/pinot/pull/13231?src=pr&el=tree&filepath=pinot-segment-spi%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Fspi%2FAggregationFunctionType.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL0FnZ3JlZ2F0aW9uRnVuY3Rpb25UeXBlLmphdmE=) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/13231?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...gregation/function/AggregationFunctionFactory.java](https://app.codecov.io/gh/apache/pinot/pull/13231?src=pr&el=tree&filepath=pinot-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcore%2Fquery%2Faggregation%2Ffunction%2FAggregationFunctionFactory.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9BZ2dyZWdhdGlvbkZ1bmN0aW9uRmFjdG9yeS5qYXZh) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/13231?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   
   <details><summary>Additional details and impacted files</summary>
   
   
   ```diff
   @@              Coverage Diff              @@
   ##             master   #13231       +/-   ##
   =============================================
   - Coverage     61.75%    0.00%   -61.76%     
   =============================================
     Files          2436     2461       +25     
     Lines        133233   135394     +2161     
     Branches      20636    20997      +361     
   =============================================
   - Hits          82274        0    -82274     
   - Misses        44911   135394    +90483     
   + Partials       6048        0     -6048     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/pinot/pull/13231/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/13231/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [integration](https://app.codecov.io/gh/apache/pinot/pull/13231/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <0.00%> (-0.01%)` | :arrow_down: |
   | [integration1](https://app.codecov.io/gh/apache/pinot/pull/13231/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [integration2](https://app.codecov.io/gh/apache/pinot/pull/13231/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <0.00%> (ø)` | |
   | [java-11](https://app.codecov.io/gh/apache/pinot/pull/13231/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [java-21](https://app.codecov.io/gh/apache/pinot/pull/13231/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <0.00%> (-61.63%)` | :arrow_down: |
   | [skip-bytebuffers-false](https://app.codecov.io/gh/apache/pinot/pull/13231/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <0.00%> (-61.75%)` | :arrow_down: |
   | [skip-bytebuffers-true](https://app.codecov.io/gh/apache/pinot/pull/13231/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [temurin](https://app.codecov.io/gh/apache/pinot/pull/13231/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <0.00%> (-61.76%)` | :arrow_down: |
   | [unittests](https://app.codecov.io/gh/apache/pinot/pull/13231/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [unittests1](https://app.codecov.io/gh/apache/pinot/pull/13231/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [unittests2](https://app.codecov.io/gh/apache/pinot/pull/13231/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/pinot/pull/13231?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Add FunnelCompleteCount aggregation function [pinot]

Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #13231:
URL: https://github.com/apache/pinot/pull/13231#discussion_r1623897488


##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/funnel/window/FunnelBaseAggregationFunction.java:
##########
@@ -32,33 +32,34 @@
 import org.apache.pinot.core.query.aggregation.AggregationResultHolder;
 import org.apache.pinot.core.query.aggregation.ObjectAggregationResultHolder;
 import org.apache.pinot.core.query.aggregation.function.AggregationFunction;
+import org.apache.pinot.core.query.aggregation.function.funnel.FunnelStepEvent;
 import org.apache.pinot.core.query.aggregation.groupby.GroupByResultHolder;
 import org.apache.pinot.core.query.aggregation.groupby.ObjectGroupByResultHolder;
-import org.apache.pinot.segment.spi.AggregationFunctionType;
 
 
-public class FunnelMaxStepAggregationFunction implements AggregationFunction<PriorityQueue<FunnelStepEvent>, Long> {
-  private final ExpressionContext _timestampExpression;
-  private final long _windowSize;
-  private final List<ExpressionContext> _stepExpressions;
-  private final FunnelModes _modes = new FunnelModes();
-  private final int _numSteps;
+public abstract class FunnelBaseAggregationFunction<F extends Comparable>
+    implements AggregationFunction<PriorityQueue<FunnelStepEvent>, F> {
+  protected final ExpressionContext _timestampExpression;
+  protected final long _windowSize;
+  protected final List<ExpressionContext> _stepExpressions;
+  protected final FunnelModes _modes = new FunnelModes();
+  protected final int _numSteps;
 
-  public FunnelMaxStepAggregationFunction(List<ExpressionContext> arguments) {
+  public FunnelBaseAggregationFunction(List<ExpressionContext> arguments) {
     int numArguments = arguments.size();
     Preconditions.checkArgument(numArguments > 2,
-        "FUNNELMAXSTEP expects >= 3 arguments, got: %s. The function can be used as "
+        "FUNNEL_BASE_FUNC expects >= 3 arguments, got: %s. The function can be used as "
             + "funnelMaxStep(timestampExpression, windowSize, ARRAY[stepExpression, ..], [mode, [mode, ... ]])",
         numArguments);
     _timestampExpression = arguments.get(0);
     _windowSize = arguments.get(1).getLiteral().getLongValue();
     Preconditions.checkArgument(_windowSize > 0, "Window size must be > 0");
     ExpressionContext stepExpressionContext = arguments.get(2);
     if (stepExpressionContext.getFunction() != null) {
-      // LEAF stage init this function like funnelmaxstep($1,'1000',arrayValueConstructor($2,$3,$4,...))
+      // LEAF stage init this function like funnelBaseFunc($1,'1000',arrayValueConstructor($2,$3,$4,...))
       _stepExpressions = stepExpressionContext.getFunction().getArguments();
     } else {
-      // Intermediate Stage init this function like funnelmaxstep($1,'1000',__PLACEHOLDER__)
+      // Intermediate Stage init this function like funnelBaseFunc($1,'1000',__PLACEHOLDER__)

Review Comment:
   added test for this 



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Add FunnelCompleteCount aggregation function [pinot]

Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 merged PR #13231:
URL: https://github.com/apache/pinot/pull/13231


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Add FunnelCompleteCount aggregation function [pinot]

Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #13231:
URL: https://github.com/apache/pinot/pull/13231#discussion_r1623651856


##########
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/ArrayFunctions.java:
##########
@@ -230,6 +230,42 @@ public static String arrayElementAtString(String[] arr, int idx) {
     return idx > 0 && idx <= arr.length ? arr[idx - 1] : NullValuePlaceHolder.STRING;
   }
 
+  @ScalarFunction
+  public static int arraySumInt(int[] arr) {
+    int sum = 0;
+    for (int value : arr) {
+      sum += value;
+    }
+    return sum;
+  }
+
+  @ScalarFunction
+  public static long arraySumLong(long[] arr) {
+    long sum = 0;
+    for (long value : arr) {
+      sum += value;
+    }
+    return sum;
+  }
+
+  @ScalarFunction
+  public static double arraySumFloat(float[] arr) {

Review Comment:
   removed this function, it can be replaced by `arraySum`



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org