Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
Drill >> mail # dev >> [3/3] git commit: Update AggregateChecker to add errors to ErrorCollector instead of throwing Exceptions 1. Tests pending


Copy link to this message
-
[3/3] git commit: Update AggregateChecker to add errors to ErrorCollector instead of throwing Exceptions 1. Tests pending
Update AggregateChecker to add errors to ErrorCollector instead of throwing Exceptions
1. Tests pending
Project: http://git-wip-us.apache.org/repos/asf/incubator-drill/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-drill/commit/4e817d11
Tree: http://git-wip-us.apache.org/repos/asf/incubator-drill/tree/4e817d11
Diff: http://git-wip-us.apache.org/repos/asf/incubator-drill/diff/4e817d11

Branch: refs/heads/master
Commit: 4e817d115a0c86e6491b453d962be16aa7f7985e
Parents: 356c2db
Author: vkorukanti <[EMAIL PROTECTED]>
Authored: Fri Mar 28 06:18:13 2014 -0700
Committer: Jacques Nadeau <[EMAIL PROTECTED]>
Committed: Sat Mar 29 14:43:48 2014 -0700

 .../expression/visitors/AggregateChecker.java   | 59 ++++++++++----------
 .../visitors/ExpressionValidator.java           |  2 +-
 2 files changed, 30 insertions(+), 31 deletions(-)
http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/4e817d11/common/src/main/java/org/apache/drill/common/expression/visitors/AggregateChecker.java
diff --git a/common/src/main/java/org/apache/drill/common/expression/visitors/AggregateChecker.java b/common/src/main/java/org/apache/drill/common/expression/visitors/AggregateChecker.java
index 630d00a..100bf94 100644
+++ b/common/src/main/java/org/apache/drill/common/expression/visitors/AggregateChecker.java
@@ -18,6 +18,7 @@
 package org.apache.drill.common.expression.visitors;
 
 import org.apache.drill.common.expression.CastExpression;
+import org.apache.drill.common.expression.ErrorCollector;
 import org.apache.drill.common.expression.FunctionCall;
 import org.apache.drill.common.expression.FunctionHolderExpression;
 import org.apache.drill.common.expression.IfExpression;
@@ -31,92 +32,90 @@ import org.apache.drill.common.expression.ValueExpressions.FloatExpression;
 import org.apache.drill.common.expression.ValueExpressions.IntExpression;
 import org.apache.drill.common.expression.ValueExpressions.QuotedString;
 
-public final class AggregateChecker extends SimpleExprVisitor<Boolean>{
-
+public final class AggregateChecker implements ExprVisitor<Boolean, ErrorCollector, RuntimeException>{
+
   public static final AggregateChecker INSTANCE = new AggregateChecker();
-  
-  private AggregateChecker(){};
-  
-  public static boolean isAggregating(LogicalExpression e){
-    return e.accept(INSTANCE, null);
+
+  public static boolean isAggregating(LogicalExpression e, ErrorCollector errors){
+    return e.accept(INSTANCE, errors);
   }
 
   @Override
-  public Boolean visitFunctionCall(FunctionCall call) {
+  public Boolean visitFunctionCall(FunctionCall call, ErrorCollector errors) {
     throw new UnsupportedOperationException("FunctionCall is not expected here. "+
       "It should have been converted to FunctionHolderExpression in materialization");
   }
 
   @Override
-  public Boolean visitFunctionHolderExpression(FunctionHolderExpression holder) throws RuntimeException {
+  public Boolean visitFunctionHolderExpression(FunctionHolderExpression holder, ErrorCollector errors) {
     if(holder.isAggregating()){
-      for(LogicalExpression e : holder.args){
-        if(e.accept(this, null))
-          throw new IllegalArgumentException(String.format(
-            "Your aggregating function call %s also includes arguments that contain aggregations."+
-            " This isn't allowed.", holder.getName()));
+      for (int i = 0; i < holder.args.size(); i++) {
+        LogicalExpression e = holder.args.get(i);
+        if(e.accept(this, errors)){
+          errors.addGeneralError(e.getPosition(),
+            String.format("Aggregating function call %s includes nested aggregations at arguments number %d. " +
+              "This isn't allowed.", holder.getName(), i));
+        }
       }
       return true;
     }else{
       for(LogicalExpression e : holder.args){
-        if(e.accept(this, null)) return true;
+        if(e.accept(this, errors)) return true;
       }
       return false;
     }
   }
 
   @Override
-  public Boolean visitIfExpression(IfExpression ifExpr) {
+  public Boolean visitIfExpression(IfExpression ifExpr, ErrorCollector errors) {
     for(IfCondition c : ifExpr.conditions){
-      if(c.condition.accept(this, null) || c.expression.accept(this, null)) return true;
+      if(c.condition.accept(this, errors) || c.expression.accept(this, errors)) return true;
     }
-    return ifExpr.elseExpression.accept(this, null);
+    return ifExpr.elseExpression.accept(this, errors);
   }
 
   @Override
-  public Boolean visitSchemaPath(SchemaPath path) {
+  public Boolean visitSchemaPath(SchemaPath path, ErrorCollector errors) {
     return false;
   }
 
   @Override
-  public Boolean visitIntConstant(IntExpression intExpr) {
+  public Boolean visitIntConstant(IntExpression intExpr, ErrorCollector errors) {
     return false;
   }
 
   @Override
-  public Boolean visitFloatConstant(FloatExpression fExpr) {
+  public Boolean visitFloatConstant(FloatExpression fExpr, ErrorCollector errors) {
     return false;
   }
 
   @Override
-  public Boolean visitLongConstant(LongExpression intExpr) {
+  public Boolean visitLongConstant(LongExpression intExpr, ErrorCollector errors) {
     return false;
   }
 
   @Override
-  public Boolean visitDoubleConstant(DoubleExpression dExpr) {
+  public Boolean visitDoubleConstant(DoubleExpression dExpr, ErrorCollector errors) {
     return false;
   }
 
   @Override
-  public Boolean visitBooleanConstant(BooleanExpression e) {
+  public Boolean visitBooleanConstant(BooleanExpression e, ErrorCollector errors) {
     return false;
   }
 
   @Override
-  public Boolean visitQuotedStringConstant(QuotedString e) {
+  public Boolean visitQuotedStringConstant(QuotedString e, ErrorCollector errors) {
     return false;
   }
 
   @Override
-  public Boolean visitUnknown(LogicalExpression e, Void value) throws RuntimeException {
+  public Boolean visitUnknown(LogicalExpression e, ErrorCollector errors) {
     return false;
   }
 
   @Override
-  public Bool