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-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
