diff --git a/client/src/main/java/com/metamx/druid/client/CachingClusteredClient.java b/client/src/main/java/com/metamx/druid/client/CachingClusteredClient.java index c49e095d2ad..47376dd2e68 100644 --- a/client/src/main/java/com/metamx/druid/client/CachingClusteredClient.java +++ b/client/src/main/java/com/metamx/druid/client/CachingClusteredClient.java @@ -24,6 +24,7 @@ import com.google.common.base.Supplier; import com.google.common.base.Throwables; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; +import com.google.common.collect.Iterators; import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.common.collect.Ordering; @@ -237,6 +238,10 @@ public class CachingClusteredClient implements QueryRunner public Iterator make() { try { + if (cachedResult.length == 0) { + return Iterators.emptyIterator(); + } + return objectMapper.readValues( objectMapper.getJsonFactory().createJsonParser(cachedResult), Object.class ); diff --git a/client/src/main/java/com/metamx/druid/query/Queries.java b/client/src/main/java/com/metamx/druid/query/Queries.java new file mode 100644 index 00000000000..2fb17b2fe76 --- /dev/null +++ b/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 aggFactories, + List 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 combinedAggNames = Sets.newHashSet( + Lists.transform( + aggFactories, + new Function() + { + @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()); + } + } + } +} diff --git a/client/src/main/java/com/metamx/druid/query/timeboundary/TimeBoundaryQuery.java b/client/src/main/java/com/metamx/druid/query/timeboundary/TimeBoundaryQuery.java index b9b64fe6948..541d5342c73 100644 --- a/client/src/main/java/com/metamx/druid/query/timeboundary/TimeBoundaryQuery.java +++ b/client/src/main/java/com/metamx/druid/query/timeboundary/TimeBoundaryQuery.java @@ -42,7 +42,10 @@ import java.util.Map; */ public class TimeBoundaryQuery extends BaseQuery> { - public static final Interval MY_Y2K_INTERVAL = new Interval(new DateTime(Long.MIN_VALUE), new DateTime(Long.MAX_VALUE)); + public static final Interval MY_Y2K_INTERVAL = new Interval( + new DateTime(Long.MIN_VALUE), + new DateTime(Long.MAX_VALUE) + ); public static final String MAX_TIME = "maxTime"; public static final String MIN_TIME = "minTime"; private static final byte CACHE_TYPE_ID = 0x0; @@ -131,6 +134,10 @@ public class TimeBoundaryQuery extends BaseQuery public Iterable> mergeResults(List> results) { + if (results == null || results.isEmpty()) { + return Lists.newArrayList(); + } + DateTime min = new DateTime(Long.MAX_VALUE); DateTime max = new DateTime(Long.MIN_VALUE); for (Result result : results) { diff --git a/client/src/main/java/com/metamx/druid/query/timeseries/TimeseriesQuery.java b/client/src/main/java/com/metamx/druid/query/timeseries/TimeseriesQuery.java index 3c664402df3..490e2531edd 100644 --- a/client/src/main/java/com/metamx/druid/query/timeseries/TimeseriesQuery.java +++ b/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> this.aggregatorSpecs = aggregatorSpecs; this.postAggregatorSpecs = postAggregatorSpecs == null ? ImmutableList.of() : postAggregatorSpecs; - Preconditions.checkNotNull(aggregatorSpecs, "aggregations cannot be null"); - Preconditions.checkArgument(aggregatorSpecs.size() > 0, "Must have at least one AggregatorFactory"); + Queries.verifyAggregations(this.aggregatorSpecs, this.postAggregatorSpecs); } @Override diff --git a/client/src/test/java/com/metamx/druid/query/QueriesTest.java b/client/src/test/java/com/metamx/druid/query/QueriesTest.java new file mode 100644 index 00000000000..7463efdc58b --- /dev/null +++ b/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 aggFactories = Arrays.asList( + new CountAggregatorFactory("count"), + new DoubleSumAggregatorFactory("idx", "index"), + new DoubleSumAggregatorFactory("rev", "revenue") + ); + + List postAggs = Arrays.asList( + new ArithmeticPostAggregator( + "addStuff", + "+", + Arrays.asList( + new FieldAccessPostAggregator("idx", "idx"), + new FieldAccessPostAggregator("count", "count") + ) + ) + ); + + boolean exceptionOccured = false; + + try { + Queries.verifyAggregations(aggFactories, postAggs); + } + catch (IllegalArgumentException e) { + exceptionOccured = true; + } + + Assert.assertFalse(exceptionOccured); + } + + @Test + public void testVerifyAggregationsMissingVal() throws Exception + { + List aggFactories = Arrays.asList( + new CountAggregatorFactory("count"), + new DoubleSumAggregatorFactory("idx", "index"), + new DoubleSumAggregatorFactory("rev", "revenue") + ); + + List postAggs = Arrays.asList( + new ArithmeticPostAggregator( + "addStuff", + "+", + Arrays.asList( + new FieldAccessPostAggregator("idx", "idx2"), + new FieldAccessPostAggregator("count", "count") + ) + ) + ); + + boolean exceptionOccured = false; + + try { + Queries.verifyAggregations(aggFactories, postAggs); + } + catch (IllegalArgumentException e) { + exceptionOccured = true; + } + + Assert.assertTrue(exceptionOccured); + } + + @Test + public void testVerifyAggregationsMultiLevel() throws Exception + { + List aggFactories = Arrays.asList( + new CountAggregatorFactory("count"), + new DoubleSumAggregatorFactory("idx", "index"), + new DoubleSumAggregatorFactory("rev", "revenue") + ); + + List postAggs = Arrays.asList( + new ArithmeticPostAggregator( + "divideStuff", + "/", + Arrays.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.asList( + new FieldAccessPostAggregator("divideStuff", "divideStuff"), + new FieldAccessPostAggregator("count", "count") + ) + ) + ); + + boolean exceptionOccured = false; + + try { + Queries.verifyAggregations(aggFactories, postAggs); + } + catch (IllegalArgumentException e) { + exceptionOccured = true; + } + + Assert.assertFalse(exceptionOccured); + } + + @Test + public void testVerifyAggregationsMultiLevelMissingVal() throws Exception + { + List aggFactories = Arrays.asList( + new CountAggregatorFactory("count"), + new DoubleSumAggregatorFactory("idx", "index"), + new DoubleSumAggregatorFactory("rev", "revenue") + ); + + List postAggs = Arrays.asList( + new ArithmeticPostAggregator( + "divideStuff", + "/", + Arrays.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.asList( + new FieldAccessPostAggregator("divideStuff", "divideStuff"), + new FieldAccessPostAggregator("count", "count") + ) + ) + ); + + boolean exceptionOccured = false; + + try { + Queries.verifyAggregations(aggFactories, postAggs); + } + catch (IllegalArgumentException e) { + exceptionOccured = true; + } + + Assert.assertTrue(exceptionOccured); + } +} diff --git a/common/src/main/java/com/metamx/druid/aggregation/post/ArithmeticPostAggregator.java b/common/src/main/java/com/metamx/druid/aggregation/post/ArithmeticPostAggregator.java index bffc0f2d7bb..035d0fa6652 100644 --- a/common/src/main/java/com/metamx/druid/aggregation/post/ArithmeticPostAggregator.java +++ b/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 fieldNames) + { + for (PostAggregator field : fields) { + if (!field.verifyFields(fieldNames)) { + return false; + } + } + return true; + } + @Override public Comparator getComparator() { diff --git a/common/src/main/java/com/metamx/druid/aggregation/post/ConstantPostAggregator.java b/common/src/main/java/com/metamx/druid/aggregation/post/ConstantPostAggregator.java index eef818db49f..645de6c9b80 100644 --- a/common/src/main/java/com/metamx/druid/aggregation/post/ConstantPostAggregator.java +++ b/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 fields) + { + return true; + } + @Override public Comparator getComparator() { diff --git a/common/src/main/java/com/metamx/druid/aggregation/post/FieldAccessPostAggregator.java b/common/src/main/java/com/metamx/druid/aggregation/post/FieldAccessPostAggregator.java index 1c3f328fa87..0a1c866d044 100644 --- a/common/src/main/java/com/metamx/druid/aggregation/post/FieldAccessPostAggregator.java +++ b/common/src/main/java/com/metamx/druid/aggregation/post/FieldAccessPostAggregator.java @@ -19,11 +19,13 @@ package com.metamx.druid.aggregation.post; +import com.metamx.common.ISE; import org.codehaus.jackson.annotate.JsonCreator; import org.codehaus.jackson.annotate.JsonProperty; import java.util.Comparator; import java.util.Map; +import java.util.Set; /** */ @@ -42,6 +44,12 @@ public class FieldAccessPostAggregator implements PostAggregator this.fieldName = fieldName; } + @Override + public boolean verifyFields(Set fieldNames) + { + return fieldNames.contains(fieldName); + } + @Override public Comparator getComparator() { diff --git a/common/src/main/java/com/metamx/druid/aggregation/post/PostAggregator.java b/common/src/main/java/com/metamx/druid/aggregation/post/PostAggregator.java index 5e679d64868..487ac30efb3 100644 --- a/common/src/main/java/com/metamx/druid/aggregation/post/PostAggregator.java +++ b/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 fieldNames); + public Comparator getComparator(); public Object compute(Map combinedAggregators);