From 32ad26210dd229f1853a16696642e7b0d1b44daa Mon Sep 17 00:00:00 2001 From: nishantmonu51 Date: Tue, 18 Mar 2014 20:34:49 +0530 Subject: [PATCH] Fix backwards compatibilty constant post aggregator --- .../post/ConstantPostAggregator.java | 8 +++-- .../test/java/io/druid/query/QueriesTest.java | 8 ++--- .../io/druid/query/QueryRunnerTestHelper.java | 2 +- .../post/ArithmeticPostAggregatorTest.java | 4 +-- .../post/ConstantPostAggregatorTest.java | 34 ++++++++++++++++--- .../timeseries/TimeseriesBinaryFnTest.java | 2 +- .../io/druid/query/topn/TopNBinaryFnTest.java | 2 +- .../antlr4/io/druid/sql/antlr4/DruidSQL.g4 | 6 ++-- 8 files changed, 48 insertions(+), 18 deletions(-) diff --git a/processing/src/main/java/io/druid/query/aggregation/post/ConstantPostAggregator.java b/processing/src/main/java/io/druid/query/aggregation/post/ConstantPostAggregator.java index ce8277f8434..6bbacdeba48 100644 --- a/processing/src/main/java/io/druid/query/aggregation/post/ConstantPostAggregator.java +++ b/processing/src/main/java/io/druid/query/aggregation/post/ConstantPostAggregator.java @@ -21,6 +21,7 @@ package io.druid.query.aggregation.post; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.base.Preconditions; import com.google.common.collect.Sets; import io.druid.query.aggregation.PostAggregator; @@ -38,11 +39,13 @@ public class ConstantPostAggregator implements PostAggregator @JsonCreator public ConstantPostAggregator( @JsonProperty("name") String name, - @JsonProperty("value") Number constantValue + @JsonProperty("value") Number constantValue, + @JsonProperty("constantValue") Number backwardsCompatibleValue ) { this.name = name; - this.constantValue = constantValue; + this.constantValue = constantValue == null ? backwardsCompatibleValue : constantValue; + Preconditions.checkNotNull(this.constantValue); } @Override @@ -126,4 +129,5 @@ public class ConstantPostAggregator implements PostAggregator result = 31 * result + (constantValue != null ? constantValue.hashCode() : 0); return result; } + } diff --git a/processing/src/test/java/io/druid/query/QueriesTest.java b/processing/src/test/java/io/druid/query/QueriesTest.java index eaac2446d76..763aeb135fd 100644 --- a/processing/src/test/java/io/druid/query/QueriesTest.java +++ b/processing/src/test/java/io/druid/query/QueriesTest.java @@ -119,7 +119,7 @@ public class QueriesTest "+", Arrays.asList( new FieldAccessPostAggregator("idx", "idx"), - new ConstantPostAggregator("const", 1) + new ConstantPostAggregator("const", 1, null) ) ), new ArithmeticPostAggregator( @@ -127,7 +127,7 @@ public class QueriesTest "-", Arrays.asList( new FieldAccessPostAggregator("rev", "rev"), - new ConstantPostAggregator("const", 1) + new ConstantPostAggregator("const", 1, null) ) ) ) @@ -173,7 +173,7 @@ public class QueriesTest "+", Arrays.asList( new FieldAccessPostAggregator("idx", "idx"), - new ConstantPostAggregator("const", 1) + new ConstantPostAggregator("const", 1, null) ) ), new ArithmeticPostAggregator( @@ -181,7 +181,7 @@ public class QueriesTest "-", Arrays.asList( new FieldAccessPostAggregator("rev", "rev2"), - new ConstantPostAggregator("const", 1) + new ConstantPostAggregator("const", 1, null) ) ) ) diff --git a/processing/src/test/java/io/druid/query/QueryRunnerTestHelper.java b/processing/src/test/java/io/druid/query/QueryRunnerTestHelper.java index 519c7375b10..7ca66b53d7e 100644 --- a/processing/src/test/java/io/druid/query/QueryRunnerTestHelper.java +++ b/processing/src/test/java/io/druid/query/QueryRunnerTestHelper.java @@ -67,7 +67,7 @@ public class QueryRunnerTestHelper "uniques", "quality_uniques" ); - public static final ConstantPostAggregator constant = new ConstantPostAggregator("const", 1L); + public static final ConstantPostAggregator constant = new ConstantPostAggregator("const", 1L, null); public static final FieldAccessPostAggregator rowsPostAgg = new FieldAccessPostAggregator("rows", "rows"); public static final FieldAccessPostAggregator indexPostAgg = new FieldAccessPostAggregator("index", "index"); public static final ArithmeticPostAggregator addRowsIndexConstant = diff --git a/processing/src/test/java/io/druid/query/aggregation/post/ArithmeticPostAggregatorTest.java b/processing/src/test/java/io/druid/query/aggregation/post/ArithmeticPostAggregatorTest.java index 6c0f49e96bd..e56ccb94ad9 100644 --- a/processing/src/test/java/io/druid/query/aggregation/post/ArithmeticPostAggregatorTest.java +++ b/processing/src/test/java/io/druid/query/aggregation/post/ArithmeticPostAggregatorTest.java @@ -48,7 +48,7 @@ public class ArithmeticPostAggregatorTest List postAggregatorList = Lists.newArrayList( new ConstantPostAggregator( - "roku", 6 + "roku", 6, null ), new FieldAccessPostAggregator( "rows", "rows" @@ -79,7 +79,7 @@ public class ArithmeticPostAggregatorTest List postAggregatorList = Lists.newArrayList( new ConstantPostAggregator( - "roku", 6 + "roku", 6, null ), new FieldAccessPostAggregator( "rows", "rows" diff --git a/processing/src/test/java/io/druid/query/aggregation/post/ConstantPostAggregatorTest.java b/processing/src/test/java/io/druid/query/aggregation/post/ConstantPostAggregatorTest.java index 08d86452230..68f0f54efdb 100644 --- a/processing/src/test/java/io/druid/query/aggregation/post/ConstantPostAggregatorTest.java +++ b/processing/src/test/java/io/druid/query/aggregation/post/ConstantPostAggregatorTest.java @@ -19,6 +19,7 @@ package io.druid.query.aggregation.post; +import io.druid.jackson.DefaultObjectMapper; import org.junit.Assert; import org.junit.Test; @@ -33,11 +34,11 @@ public class ConstantPostAggregatorTest { ConstantPostAggregator constantPostAggregator; - constantPostAggregator = new ConstantPostAggregator("shichi", 7); + constantPostAggregator = new ConstantPostAggregator("shichi", 7, null); Assert.assertEquals(7, constantPostAggregator.compute(null)); - constantPostAggregator = new ConstantPostAggregator("rei", 0.0); + constantPostAggregator = new ConstantPostAggregator("rei", 0.0, null); Assert.assertEquals(0.0, constantPostAggregator.compute(null)); - constantPostAggregator = new ConstantPostAggregator("ichi", 1.0); + constantPostAggregator = new ConstantPostAggregator("ichi", 1.0, null); Assert.assertNotSame(1, constantPostAggregator.compute(null)); } @@ -45,10 +46,35 @@ public class ConstantPostAggregatorTest public void testComparator() { ConstantPostAggregator constantPostAggregator = - new ConstantPostAggregator("thistestbasicallydoesnothing unhappyface", 1); + new ConstantPostAggregator("thistestbasicallydoesnothing unhappyface", 1, null); Comparator comp = constantPostAggregator.getComparator(); Assert.assertEquals(0, comp.compare(0, constantPostAggregator.compute(null))); Assert.assertEquals(0, comp.compare(0, 1)); Assert.assertEquals(0, comp.compare(1, 0)); } + + @Test + public void testSerdeBackwardsCompatible() throws Exception + { + + DefaultObjectMapper mapper = new DefaultObjectMapper(); + ConstantPostAggregator aggregator = mapper.readValue( + "{\"type\":\"constant\",\"name\":\"thistestbasicallydoesnothing unhappyface\",\"constantValue\":1}\n", + ConstantPostAggregator.class + ); + Assert.assertEquals(new Integer(1), aggregator.getConstantValue()); + } + + @Test + public void testSerde() throws Exception + { + + DefaultObjectMapper mapper = new DefaultObjectMapper(); + ConstantPostAggregator aggregator = new ConstantPostAggregator("aggregator", 2, null); + ConstantPostAggregator aggregator1 = mapper.readValue( + mapper.writeValueAsString(aggregator), + ConstantPostAggregator.class + ); + Assert.assertEquals(aggregator, aggregator1); + } } diff --git a/processing/src/test/java/io/druid/query/timeseries/TimeseriesBinaryFnTest.java b/processing/src/test/java/io/druid/query/timeseries/TimeseriesBinaryFnTest.java index 03211178d25..e574cbd4b44 100644 --- a/processing/src/test/java/io/druid/query/timeseries/TimeseriesBinaryFnTest.java +++ b/processing/src/test/java/io/druid/query/timeseries/TimeseriesBinaryFnTest.java @@ -43,7 +43,7 @@ public class TimeseriesBinaryFnTest { final CountAggregatorFactory rowsCount = new CountAggregatorFactory("rows"); final LongSumAggregatorFactory indexLongSum = new LongSumAggregatorFactory("index", "index"); - final ConstantPostAggregator constant = new ConstantPostAggregator("const", 1L); + final ConstantPostAggregator constant = new ConstantPostAggregator("const", 1L, null); final FieldAccessPostAggregator rowsPostAgg = new FieldAccessPostAggregator("rows", "rows"); final FieldAccessPostAggregator indexPostAgg = new FieldAccessPostAggregator("index", "index"); final ArithmeticPostAggregator addRowsIndexConstant = new ArithmeticPostAggregator( diff --git a/processing/src/test/java/io/druid/query/topn/TopNBinaryFnTest.java b/processing/src/test/java/io/druid/query/topn/TopNBinaryFnTest.java index 2ce63a693e2..0eba9778c4e 100644 --- a/processing/src/test/java/io/druid/query/topn/TopNBinaryFnTest.java +++ b/processing/src/test/java/io/druid/query/topn/TopNBinaryFnTest.java @@ -47,7 +47,7 @@ public class TopNBinaryFnTest { final CountAggregatorFactory rowsCount = new CountAggregatorFactory("rows"); final LongSumAggregatorFactory indexLongSum = new LongSumAggregatorFactory("index", "index"); - final ConstantPostAggregator constant = new ConstantPostAggregator("const", 1L); + final ConstantPostAggregator constant = new ConstantPostAggregator("const", 1L, null); final FieldAccessPostAggregator rowsPostAgg = new FieldAccessPostAggregator("rows", "rows"); final FieldAccessPostAggregator indexPostAgg = new FieldAccessPostAggregator("index", "index"); final ArithmeticPostAggregator addrowsindexconstant = new ArithmeticPostAggregator( diff --git a/server/src/main/antlr4/io/druid/sql/antlr4/DruidSQL.g4 b/server/src/main/antlr4/io/druid/sql/antlr4/DruidSQL.g4 index 1cafead058d..37716ab6673 100644 --- a/server/src/main/antlr4/io/druid/sql/antlr4/DruidSQL.g4 +++ b/server/src/main/antlr4/io/druid/sql/antlr4/DruidSQL.g4 @@ -212,12 +212,12 @@ unaryExpression returns [PostAggregator p] if($e.p instanceof ConstantPostAggregator) { ConstantPostAggregator c = (ConstantPostAggregator)$e.p; double v = c.getConstantValue().doubleValue() * -1; - $p = new ConstantPostAggregator(Double.toString(v), v); + $p = new ConstantPostAggregator(Double.toString(v), v, null); } else { $p = new ArithmeticPostAggregator( "-"+$e.p.getName(), "*", - Lists.newArrayList($e.p, new ConstantPostAggregator("-1", -1.0)) + Lists.newArrayList($e.p, new ConstantPostAggregator("-1", -1.0, null)) ); } } @@ -240,7 +240,7 @@ aggregate returns [AggregatorFactory agg] ; constant returns [ConstantPostAggregator c] - : value=NUMBER { double v = Double.parseDouble($value.text); $c = new ConstantPostAggregator(Double.toString(v), v); } + : value=NUMBER { double v = Double.parseDouble($value.text); $c = new ConstantPostAggregator(Double.toString(v), v, null); } ; /* time filters must be top level filters */