You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@asterixdb.apache.org by ti...@apache.org on 2019/04/03 18:22:20 UTC

[asterixdb] branch master updated: [NO ISSUE][COMP] Multiple parameters for two-step aggregation functions

This is an automated email from the ASF dual-hosted git repository.

tillw pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/asterixdb.git


The following commit(s) were added to refs/heads/master by this push:
     new e5735e4  [NO ISSUE][COMP] Multiple parameters for two-step aggregation functions
e5735e4 is described below

commit e5735e4bb0da8e922f242fa6d666462047d28a1a
Author: sandeepgupta <sa...@couchbase.com>
AuthorDate: Wed Apr 3 00:02:24 2019 +0530

    [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 <je...@fulliautomatix.ics.uci.edu>
    Tested-by: Till Westmann <ti...@apache.org>
    Contrib: Jenkins <je...@fulliautomatix.ics.uci.edu>
---
 .../rules/PushAggFuncIntoStandaloneAggregateRule.java      | 10 ++++++----
 .../java/org/apache/asterix/test/common/TestExecutor.java  |  3 ++-
 .../scalar/AbstractScalarAggregateDescriptor.java          | 14 +++++++++++---
 .../rewriter/rules/AbstractIntroduceCombinerRule.java      |  5 ++++-
 4 files changed, 23 insertions(+), 9 deletions(-)

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 @@ public class PushAggFuncIntoStandaloneAggregateRule implements IAlgebraicRewrite
 
             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 @@ public class PushAggFuncIntoStandaloneAggregateRule implements IAlgebraicRewrite
         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 @@ public class TestExecutor {
                     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 @@ public abstract class AbstractScalarAggregateDescriptor extends AbstractScalarFu
     @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 @@ public abstract class AbstractIntroduceCombinerRule implements IAlgebraicRewrite
             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 @@ public abstract class AbstractIntroduceCombinerRule implements IAlgebraicRewrite
                     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);