Fix backwards compatibilty constant post aggregator

xvrl · Mar 18, 2014 · 32ad262 · 32ad262
1 parent 294f15d
commit 32ad262
Show file tree

Hide file tree

Showing 8 changed files with 48 additions and 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
@@ -21,6 +21,7 @@
 
 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 int hashCode()
     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
@@ -119,15 +119,15 @@ public void testVerifyAggregationsMultiLevel() throws Exception
                     "+",
                     Arrays.asList(
                         new FieldAccessPostAggregator("idx", "idx"),
-                        new ConstantPostAggregator("const", 1)
+                        new ConstantPostAggregator("const", 1, null)
                     )
                 ),
                 new ArithmeticPostAggregator(
                     "subtractStuff",
                     "-",
                     Arrays.asList(
                         new FieldAccessPostAggregator("rev", "rev"),
-                        new ConstantPostAggregator("const", 1)
+                        new ConstantPostAggregator("const", 1, null)
                     )
                 )
             )
@@ -173,15 +173,15 @@ public void testVerifyAggregationsMultiLevelMissingVal() throws Exception
                     "+",
                     Arrays.asList(
                         new FieldAccessPostAggregator("idx", "idx"),
-                        new ConstantPostAggregator("const", 1)
+                        new ConstantPostAggregator("const", 1, null)
                     )
                 ),
                 new ArithmeticPostAggregator(
                     "subtractStuff",
                     "-",
                     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
@@ -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
@@ -48,7 +48,7 @@ public void testCompute()
     List<PostAggregator> postAggregatorList =
         Lists.newArrayList(
             new ConstantPostAggregator(
-                "roku", 6
+                "roku", 6, null
             ),
             new FieldAccessPostAggregator(
                 "rows", "rows"
@@ -79,7 +79,7 @@ public void testComparator()
     List<PostAggregator> 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
@@ -19,6 +19,7 @@
 
 package io.druid.query.aggregation.post;
 
+import io.druid.jackson.DefaultObjectMapper;
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -33,22 +34,47 @@ public void testCompute()
   {
     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));
   }
 
   @Test
   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
@@ -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
@@ -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
@@ -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 */