post aggs verify that they have all the fields they need

client/src/main/java/com/metamx/druid/query/Queries.java

@@ -0,0 +1,50 @@
+package com.metamx.druid.query;
+
+import com.google.common.base.Function;
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
+import com.metamx.druid.aggregation.AggregatorFactory;
+import com.metamx.druid.aggregation.post.PostAggregator;
+
+import javax.annotation.Nullable;
+import java.util.List;
+import java.util.Set;
+
+/**
+ */
+public class Queries
+{
+  public static void verifyAggregations(
+      List<AggregatorFactory> aggFactories,
+      List<PostAggregator> postAggs
+  )
+  {
+    Preconditions.checkNotNull(aggFactories, "aggregations cannot be null");
+    Preconditions.checkArgument(aggFactories.size() > 0, "Must have at least one AggregatorFactory");
+
+    if (postAggs != null && !postAggs.isEmpty()) {
+      Set<String> combinedAggNames = Sets.newHashSet(
+          Lists.transform(
+              aggFactories,
+              new Function<AggregatorFactory, String>()
+              {
+                @Override
+                public String apply(@Nullable AggregatorFactory input)
+                {
+                  return input.getName();
+                }
+              }
+          )
+      );
+
+      for (PostAggregator postAgg : postAggs) {
+        Preconditions.checkArgument(
+            postAgg.verifyFields(combinedAggNames),
+            String.format("Missing field[%s]", postAgg.getName())
+        );
+        combinedAggNames.add(postAgg.getName());
+      }
+    }
+  }
+}

client/src/main/java/com/metamx/druid/query/timeseries/TimeseriesQuery.java

@@ -26,6 +26,7 @@ import com.metamx.druid.Query;
 import com.metamx.druid.QueryGranularity;
 import com.metamx.druid.aggregation.AggregatorFactory;
 import com.metamx.druid.aggregation.post.PostAggregator;
+import com.metamx.druid.query.Queries;
 import com.metamx.druid.query.filter.DimFilter;
 import com.metamx.druid.query.segment.QuerySegmentSpec;
 import com.metamx.druid.result.Result;
@@ -67,8 +68,7 @@ public class TimeseriesQuery extends BaseQuery<Result<TimeseriesResultValue>>
     this.aggregatorSpecs = aggregatorSpecs;
     this.postAggregatorSpecs = postAggregatorSpecs == null ? ImmutableList.<PostAggregator>of() : postAggregatorSpecs;
 
-    Preconditions.checkNotNull(aggregatorSpecs, "aggregations cannot be null");
+    Queries.verifyAggregations(this.aggregatorSpecs, this.postAggregatorSpecs);
-    Preconditions.checkArgument(aggregatorSpecs.size() > 0, "Must have at least one AggregatorFactory");
   }
 
   @Override

client/src/test/java/com/metamx/druid/query/QueriesTest.java

@@ -0,0 +1,191 @@
+package com.metamx.druid.query;
+
+import com.metamx.druid.aggregation.AggregatorFactory;
+import com.metamx.druid.aggregation.CountAggregatorFactory;
+import com.metamx.druid.aggregation.DoubleSumAggregatorFactory;
+import com.metamx.druid.aggregation.post.ArithmeticPostAggregator;
+import com.metamx.druid.aggregation.post.ConstantPostAggregator;
+import com.metamx.druid.aggregation.post.FieldAccessPostAggregator;
+import com.metamx.druid.aggregation.post.PostAggregator;
+import junit.framework.Assert;
+import org.junit.Test;
+
+import java.util.Arrays;
+import java.util.List;
+
+/**
+ */
+public class QueriesTest
+{
+  @Test
+  public void testVerifyAggregations() throws Exception
+  {
+    List<AggregatorFactory> aggFactories = Arrays.<AggregatorFactory>asList(
+        new CountAggregatorFactory("count"),
+        new DoubleSumAggregatorFactory("idx", "index"),
+        new DoubleSumAggregatorFactory("rev", "revenue")
+    );
+
+    List<PostAggregator> postAggs = Arrays.<PostAggregator>asList(
+        new ArithmeticPostAggregator(
+            "addStuff",
+            "+",
+            Arrays.<PostAggregator>asList(
+                new FieldAccessPostAggregator("idx", "idx"),
+                new FieldAccessPostAggregator("count", "count")
+            )
+        )
+    );
+
+    boolean exceptionOccured = false;
+
+    try {
+      Queries.verifyAggregations(aggFactories, postAggs);
+    }
+    catch (Exception e) {
+      exceptionOccured = true;
+    }
+
+    Assert.assertFalse(exceptionOccured);
+  }
+
+  @Test
+  public void testVerifyAggregationsMissingVal() throws Exception
+  {
+    List<AggregatorFactory> aggFactories = Arrays.<AggregatorFactory>asList(
+        new CountAggregatorFactory("count"),
+        new DoubleSumAggregatorFactory("idx", "index"),
+        new DoubleSumAggregatorFactory("rev", "revenue")
+    );
+
+    List<PostAggregator> postAggs = Arrays.<PostAggregator>asList(
+        new ArithmeticPostAggregator(
+            "addStuff",
+            "+",
+            Arrays.<PostAggregator>asList(
+                new FieldAccessPostAggregator("idx", "idx2"),
+                new FieldAccessPostAggregator("count", "count")
+            )
+        )
+    );
+
+    boolean exceptionOccured = false;
+
+    try {
+      Queries.verifyAggregations(aggFactories, postAggs);
+    }
+    catch (Exception e) {
+      exceptionOccured = true;
+    }
+
+    Assert.assertTrue(exceptionOccured);
+  }
+
+  @Test
+  public void testVerifyAggregationsMultiLevel() throws Exception
+  {
+    List<AggregatorFactory> aggFactories = Arrays.<AggregatorFactory>asList(
+        new CountAggregatorFactory("count"),
+        new DoubleSumAggregatorFactory("idx", "index"),
+        new DoubleSumAggregatorFactory("rev", "revenue")
+    );
+
+    List<PostAggregator> postAggs = Arrays.<PostAggregator>asList(
+        new ArithmeticPostAggregator(
+            "divideStuff",
+            "/",
+            Arrays.<PostAggregator>asList(
+                new ArithmeticPostAggregator(
+                    "addStuff",
+                    "+",
+                    Arrays.asList(
+                        new FieldAccessPostAggregator("idx", "idx"),
+                        new ConstantPostAggregator("const", 1)
+                    )
+                ),
+                new ArithmeticPostAggregator(
+                    "subtractStuff",
+                    "-",
+                    Arrays.asList(
+                        new FieldAccessPostAggregator("rev", "rev"),
+                        new ConstantPostAggregator("const", 1)
+                    )
+                )
+            )
+        ),
+        new ArithmeticPostAggregator(
+            "addStuff",
+            "+",
+            Arrays.<PostAggregator>asList(
+                new FieldAccessPostAggregator("divideStuff", "divideStuff"),
+                new FieldAccessPostAggregator("count", "count")
+            )
+        )
+    );
+
+    boolean exceptionOccured = false;
+
+    try {
+      Queries.verifyAggregations(aggFactories, postAggs);
+    }
+    catch (Exception e) {
+      exceptionOccured = true;
+    }
+
+    Assert.assertFalse(exceptionOccured);
+  }
+
+  @Test
+  public void testVerifyAggregationsMultiLevelMissingVal() throws Exception
+  {
+    List<AggregatorFactory> aggFactories = Arrays.<AggregatorFactory>asList(
+        new CountAggregatorFactory("count"),
+        new DoubleSumAggregatorFactory("idx", "index"),
+        new DoubleSumAggregatorFactory("rev", "revenue")
+    );
+
+    List<PostAggregator> postAggs = Arrays.<PostAggregator>asList(
+        new ArithmeticPostAggregator(
+            "divideStuff",
+            "/",
+            Arrays.<PostAggregator>asList(
+                new ArithmeticPostAggregator(
+                    "addStuff",
+                    "+",
+                    Arrays.asList(
+                        new FieldAccessPostAggregator("idx", "idx"),
+                        new ConstantPostAggregator("const", 1)
+                    )
+                ),
+                new ArithmeticPostAggregator(
+                    "subtractStuff",
+                    "-",
+                    Arrays.asList(
+                        new FieldAccessPostAggregator("rev", "rev2"),
+                        new ConstantPostAggregator("const", 1)
+                    )
+                )
+            )
+        ),
+        new ArithmeticPostAggregator(
+            "addStuff",
+            "+",
+            Arrays.<PostAggregator>asList(
+                new FieldAccessPostAggregator("divideStuff", "divideStuff"),
+                new FieldAccessPostAggregator("count", "count")
+            )
+        )
+    );
+
+    boolean exceptionOccured = false;
+
+    try {
+      Queries.verifyAggregations(aggFactories, postAggs);
+    }
+    catch (Exception e) {
+      exceptionOccured = true;
+    }
+
+    Assert.assertTrue(exceptionOccured);
+  }
+}

common/src/main/java/com/metamx/druid/aggregation/post/ArithmeticPostAggregator.java

@@ -68,6 +68,17 @@ public class ArithmeticPostAggregator implements PostAggregator
     }
   }
 
+  @Override
+  public boolean verifyFields(Set<String> fieldNames)
+  {
+    for (PostAggregator field : fields) {
+      if (!field.verifyFields(fieldNames)) {
+        return false;
+      }
+    }
+    return true;
+  }
+
   @Override
   public Comparator getComparator()
   {

common/src/main/java/com/metamx/druid/aggregation/post/ConstantPostAggregator.java

@@ -24,6 +24,7 @@ import org.codehaus.jackson.annotate.JsonProperty;
 
 import java.util.Comparator;
 import java.util.Map;
+import java.util.Set;
 
 /**
  */
@@ -42,6 +43,12 @@ public class ConstantPostAggregator implements PostAggregator
     this.constantValue = constantValue;
   }
 
+  @Override
+  public boolean verifyFields(Set<String> fields)
+  {
+    return true;
+  }
+
   @Override
   public Comparator getComparator()
   {

common/src/main/java/com/metamx/druid/aggregation/post/FieldAccessPostAggregator.java

@@ -25,6 +25,7 @@ import org.codehaus.jackson.annotate.JsonProperty;
 
 import java.util.Comparator;
 import java.util.Map;
+import java.util.Set;
 
 /**
  */
@@ -43,6 +44,12 @@ public class FieldAccessPostAggregator implements PostAggregator
     this.fieldName = fieldName;
   }
 
+  @Override
+  public boolean verifyFields(Set<String> fieldNames)
+  {
+    return fieldNames.contains(fieldName);
+  }
+
   @Override
   public Comparator getComparator()
   {
@@ -52,11 +59,7 @@ public class FieldAccessPostAggregator implements PostAggregator
   @Override
   public Object compute(Map<String, Object> combinedAggregators)
   {
-    Object retVal = combinedAggregators.get(fieldName);
+    return combinedAggregators.get(fieldName);
-    if (retVal == null) {
-      throw new ISE("Mismatch! Agg[%s] was not specified!", fieldName);
-    }
-    return retVal;
   }
 
   @Override

common/src/main/java/com/metamx/druid/aggregation/post/PostAggregator.java

@@ -24,6 +24,7 @@ import org.codehaus.jackson.annotate.JsonTypeInfo;
 
 import java.util.Comparator;
 import java.util.Map;
+import java.util.Set;
 
 /**
  * Functionally similar to an Aggregator. See the Aggregator interface for more comments.
@@ -36,6 +37,8 @@ import java.util.Map;
 })
 public interface PostAggregator
 {
+  public boolean verifyFields(Set<String> fieldNames);
+
   public Comparator getComparator();
 
   public Object compute(Map<String, Object> combinedAggregators);