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