Till Westmann has submitted this change and it was merged. Change subject: [NO ISSUE][COMP] Multiple parameters for two-step aggregation functions ......................................................................
[NO ISSUE][COMP] Multiple parameters for two-step aggregation functions - user model changes: no - storage format changes: no - interface changes: yes Details: Hooks to enable aggregate functions have multiple arguments. Change-Id: Ibaef2c0b2cf858d1aa7dd0f645f773fd04a865b3 Reviewed-on: https://asterix-gerrit.ics.uci.edu/3298 Reviewed-by: Till Westmann <ti...@apache.org> Integration-Tests: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Tested-by: Till Westmann <ti...@apache.org> Contrib: Jenkins <jenk...@fulliautomatix.ics.uci.edu> --- M asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushAggFuncIntoStandaloneAggregateRule.java M asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/TestExecutor.java M asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/aggregates/scalar/AbstractScalarAggregateDescriptor.java M hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/AbstractIntroduceCombinerRule.java 4 files changed, 23 insertions(+), 9 deletions(-) Approvals: Anon. E. Moose #1000171: Till Westmann: Looks good to me, approved; Verified Jenkins: ; Verified Objections: Jenkins: Violations found diff --git a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushAggFuncIntoStandaloneAggregateRule.java b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushAggFuncIntoStandaloneAggregateRule.java index c0dc8be..4ae25e8 100644 --- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushAggFuncIntoStandaloneAggregateRule.java +++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushAggFuncIntoStandaloneAggregateRule.java @@ -218,8 +218,11 @@ List<Mutable<ILogicalExpression>> aggArgs = new ArrayList<Mutable<ILogicalExpression>>(); aggArgs.add(aggOpExpr.getArguments().get(0)); + int sz = assignFuncExpr.getArguments().size(); + aggArgs.addAll(assignFuncExpr.getArguments().subList(1, sz)); AggregateFunctionCallExpression aggFuncExpr = BuiltinFunctions.makeAggregateFunctionExpression(aggFuncIdent, aggArgs); + aggFuncExpr.setSourceLocation(assignFuncExpr.getSourceLocation()); LogicalVariable newVar = context.newVar(); aggOp.getVariables().add(newVar); @@ -241,10 +244,9 @@ for (Mutable<ILogicalExpression> exprRef : exprRefs) { ILogicalExpression expr = exprRef.getValue(); - if (expr.getExpressionTag() == LogicalExpressionTag.VARIABLE) { - if (((VariableReferenceExpression) expr).getVariableReference().equals(aggVar)) { - return false; - } + if ((expr.getExpressionTag() == LogicalExpressionTag.VARIABLE) + && ((VariableReferenceExpression) expr).getVariableReference().equals(aggVar)) { + return false; } if (expr.getExpressionTag() != LogicalExpressionTag.FUNCTION_CALL) { diff --git a/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/TestExecutor.java b/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/TestExecutor.java index d2bbc04..217b065 100644 --- a/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/TestExecutor.java +++ b/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/TestExecutor.java @@ -252,7 +252,8 @@ throw new NullPointerException("Error parsing expected or actual result file for " + scriptFile); } if (!TestHelper.equalJson(expectedJson, actualJson)) { - throw new ComparisonException("Result for " + scriptFile + " didn't match the expected JSON"); + throw new ComparisonException( + "Result for " + scriptFile + " didn't match the expected JSON" + "\n" + actualJson); } return; } diff --git a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/aggregates/scalar/AbstractScalarAggregateDescriptor.java b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/aggregates/scalar/AbstractScalarAggregateDescriptor.java index de376c6..2a311fb 100644 --- a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/aggregates/scalar/AbstractScalarAggregateDescriptor.java +++ b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/aggregates/scalar/AbstractScalarAggregateDescriptor.java @@ -43,10 +43,18 @@ @Override public IScalarEvaluatorFactory createEvaluatorFactory(final IScalarEvaluatorFactory[] args) throws AlgebricksException { - // The aggregate function will get a SingleFieldFrameTupleReference that points to the result of the ScanCollection. - // The list-item will always reside in the first field (column) of the SingleFieldFrameTupleReference. - IScalarEvaluatorFactory[] aggFuncArgs = new IScalarEvaluatorFactory[1]; + + // The aggregate function will get a SingleFieldFrameTupleReference that points to the result of the + // ScanCollection. The list-item will always reside in the first field (column) of the + // SingleFieldFrameTupleReference. + int numArgs = args.length; + IScalarEvaluatorFactory[] aggFuncArgs = new IScalarEvaluatorFactory[numArgs]; + aggFuncArgs[0] = new ColumnAccessEvalFactory(0); + + for (int i = 1; i < numArgs; ++i) { + aggFuncArgs[i] = args[i]; + } // Create aggregate function from this scalar version. final IAggregateEvaluatorFactory aggFuncFactory = aggFuncDesc.createAggregateEvaluatorFactory(aggFuncArgs); diff --git a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/AbstractIntroduceCombinerRule.java b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/AbstractIntroduceCombinerRule.java index de96cad..1c8adfb 100644 --- a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/AbstractIntroduceCombinerRule.java +++ b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/AbstractIntroduceCombinerRule.java @@ -59,8 +59,11 @@ for (AggregateExprInfo aei : sai.simAggs) { AbstractFunctionCallExpression afce = (AbstractFunctionCallExpression) aei.aggExprRef.getValue(); afce.setFunctionInfo(aei.newFunInfo); + List<Mutable<ILogicalExpression>> args = new ArrayList<Mutable<ILogicalExpression>>(); + args.addAll(afce.getArguments()); afce.getArguments().clear(); afce.getArguments().add(new MutableObject<>(sai.stepOneResult)); + afce.getArguments().addAll(args.subList(1, args.size())); } } } @@ -126,7 +129,7 @@ return new Pair<>(false, null); } Mutable<ILogicalOperator> bottomRef = inputRef; - while (bottomRef.getValue().getInputs().size() > 0) { + while (!bottomRef.getValue().getInputs().isEmpty()) { bottomRef = bottomRef.getValue().getInputs().get(0); if (!isPushableInput(bottomRef.getValue())) { return new Pair<>(false, null); -- To view, visit https://asterix-gerrit.ics.uci.edu/3298 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ibaef2c0b2cf858d1aa7dd0f645f773fd04a865b3 Gerrit-PatchSet: 7 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: sandeep.gu...@couchbase.com Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Gerrit-Reviewer: Michael Blow <mb...@apache.org> Gerrit-Reviewer: Till Westmann <ti...@apache.org>
