SQL compatible Null Handling Part 2 - Processing Layer and Druid-SQL changes

.travis.yml

@@ -33,6 +33,15 @@ matrix:
         - unset _JAVA_OPTIONS
       script: echo "MAVEN_OPTS='-Xmx512m'" > ~/.mavenrc && mvn test -B -Pparallel-test -Dmaven.fork.count=2 -pl processing
 
+      # processing module tests with SQL Compatibility enabled
+    - sudo: false
+      env:
+        - NAME="processing module test with SQL Compatibility"
+      install: echo "MAVEN_OPTS='-Xmx3000m'" > ~/.mavenrc && mvn install -q -ff -DskipTests -B
+      before_script:
+        - unset _JAVA_OPTIONS
+      script: echo "MAVEN_OPTS='-Xmx512m'" > ~/.mavenrc && mvn test -B -Pparallel-test -Dmaven.fork.count=2 -Ddruid.generic.useDefaultValueForNull=false -pl processing
+
       # server module test
     - sudo: false
       env:
@@ -43,6 +52,17 @@ matrix:
       # Server module test is run without the parallel-test option because it's memory sensitive and often fails with that option.
       script: echo "MAVEN_OPTS='-Xmx512m'" > ~/.mavenrc && mvn test -B -pl server
 
+      # server module test with SQL Compatibility enabled
+    - sudo: false
+      env:
+        - NAME="server module test with SQL Compatibility enabled"
+      install: echo "MAVEN_OPTS='-Xmx3000m'" > ~/.mavenrc && mvn install -q -ff -DskipTests -B
+      before_script:
+        - unset _JAVA_OPTIONS
+      # Server module test is run without the parallel-test option because it's memory sensitive and often fails with that option.
+      script: echo "MAVEN_OPTS='-Xmx512m'" > ~/.mavenrc && mvn test -B -pl server -Ddruid.generic.useDefaultValueForNull=false
+
+
       # other modules test
     - sudo: false
       env:
@@ -53,6 +73,16 @@ matrix:
         - unset _JAVA_OPTIONS
       script: echo "MAVEN_OPTS='-Xmx512m'" > ~/.mavenrc && mvn test -B -Pparallel-test -Dmaven.fork.count=2 -pl '!processing,!server'
 
+      # other modules test with SQL Compatibility enabled
+    - sudo: false
+      env:
+        - NAME="other modules test with SQL Compatibility"
+        - AWS_REGION=us-east-1 # set a aws region for unit tests
+      install: echo "MAVEN_OPTS='-Xmx3000m'" > ~/.mavenrc && mvn install -q -ff -DskipTests -B
+      before_script:
+        - unset _JAVA_OPTIONS
+      script: echo "MAVEN_OPTS='-Xmx512m'" > ~/.mavenrc && mvn test -B -Pparallel-test -Dmaven.fork.count=2 -Ddruid.generic.useDefaultValueForNull=false -pl '!processing,!server'
+
       # run integration tests
     - sudo: required
       services:

api/src/main/java/io/druid/data/input/Rows.java

@@ -23,9 +23,11 @@
 import com.google.common.collect.ImmutableSortedSet;
 import com.google.common.collect.Maps;
 import com.google.common.primitives.Longs;
+import io.druid.common.config.NullHandling;
 import io.druid.java.util.common.StringUtils;
 import io.druid.java.util.common.parsers.ParseException;
 
+import javax.annotation.Nullable;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
@@ -92,10 +94,11 @@ public static List<String> objectToStrings(final Object inputValue)
    * @throws NullPointerException if the string is null
    * @throws ParseException       if the column cannot be converted to a number
    */
+  @Nullable
   public static Number objectToNumber(final String name, final Object inputValue)
   {
     if (inputValue == null) {
-      return Rows.LONG_ZERO;
+      return NullHandling.defaultLongValue();
     }
 
     if (inputValue instanceof Number) {

benchmarks/src/main/java/io/druid/benchmark/FilterPartitionBenchmark.java

@@ -21,12 +21,12 @@
 
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.google.common.base.Predicate;
-import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
 import com.google.common.io.Files;
 import io.druid.benchmark.datagen.BenchmarkDataGenerator;
 import io.druid.benchmark.datagen.BenchmarkSchemaInfo;
 import io.druid.benchmark.datagen.BenchmarkSchemas;
+import io.druid.common.config.NullHandling;
 import io.druid.data.input.InputRow;
 import io.druid.hll.HyperLogLogHash;
 import io.druid.jackson.DefaultObjectMapper;
@@ -557,7 +557,7 @@ public Filter toFilter()
       if (extractionFn == null) {
         return new NoBitmapSelectorFilter(dimension, value);
       } else {
-        final String valueOrNull = Strings.emptyToNull(value);
+        final String valueOrNull = NullHandling.emptyToNullIfNeeded(value);
 
         final DruidPredicateFactory predicateFactory = new DruidPredicateFactory()
         {

codestyle/checkstyle.xml

@@ -171,5 +171,15 @@
       <property name="illegalPattern" value="true"/>
       <property name="message" value="Use java.lang.Primitive.BYTES instead."/>
     </module>
+    <module name="Regexp">
+      <property name="format" value="Strings.emptyToNull"/>
+      <property name="illegalPattern" value="true"/>
+      <property name="message" value="Use io.druid.common.config.NullHandling.emptyToNullIfNeeded instead"/>
+    </module>
+    <module name="Regexp">
+      <property name="format" value="Strings.nullToEmpty"/>
+      <property name="illegalPattern" value="true"/>
+      <property name="message" value="Use io.druid.common.config.NullHandling.nullToEmptyIfNeeded instead"/>
+    </module>
   </module>
 </module>

common/src/main/java/io/druid/math/expr/Evals.java

@@ -19,7 +19,7 @@
 
 package io.druid.math.expr;
 
-import com.google.common.base.Strings;
+import io.druid.common.config.NullHandling;
 import io.druid.java.util.common.logger.Logger;
 
 import java.util.Arrays;
@@ -83,6 +83,6 @@ public static boolean asBoolean(double x)
 
   public static boolean asBoolean(String x)
   {
-    return !Strings.isNullOrEmpty(x) && Boolean.valueOf(x);
+    return !NullHandling.isNullOrEquivalent(x) && Boolean.valueOf(x);
   }
 }

common/src/main/java/io/druid/math/expr/Expr.java

@@ -272,6 +272,9 @@ class UnaryMinusExpr extends UnaryExpr
   public ExprEval eval(ObjectBinding bindings)
   {
     ExprEval ret = expr.eval(bindings);
+    if (NullHandling.sqlCompatible() && (ret.value() == null)) {
+      return ExprEval.of(null);
+    }
     if (ret.type() == ExprType.LONG) {
       return ExprEval.of(-ret.asLong());
     }
@@ -307,6 +310,9 @@ class UnaryNotExpr extends UnaryExpr
   public ExprEval eval(ObjectBinding bindings)
   {
     ExprEval ret = expr.eval(bindings);
+    if (NullHandling.sqlCompatible() && (ret.value() == null)) {
+      return ExprEval.of(null);
+    }
     // conforming to other boolean-returning binary operators
     ExprType retType = ret.type() == ExprType.DOUBLE ? ExprType.DOUBLE : ExprType.LONG;
     return ExprEval.of(!ret.asBoolean(), retType);
@@ -365,15 +371,21 @@ public ExprEval eval(ObjectBinding bindings)
 
     // Result of any Binary expressions is null if any of the argument is null.
     // e.g "select null * 2 as c;" or "select null + 1 as c;" will return null as per Standard SQL spec.
-    if (NullHandling.sqlCompatible() && (leftVal.isNull() || rightVal.isNull())) {
+    if (NullHandling.sqlCompatible() && (leftVal.value() == null || rightVal.value() == null)) {
       return ExprEval.of(null);
     }
 
     if (leftVal.type() == ExprType.STRING && rightVal.type() == ExprType.STRING) {
       return evalString(leftVal.asString(), rightVal.asString());
     } else if (leftVal.type() == ExprType.LONG && rightVal.type() == ExprType.LONG) {
+      if (NullHandling.sqlCompatible() && (leftVal.isNumericNull() || rightVal.isNumericNull())) {
+        return ExprEval.of(null);
+      }
       return ExprEval.of(evalLong(leftVal.asLong(), rightVal.asLong()));
     } else {
+      if (NullHandling.sqlCompatible() && (leftVal.isNumericNull() || rightVal.isNumericNull())) {
+        return ExprEval.of(null);
+      }
       return ExprEval.of(evalDouble(leftVal.asDouble(), rightVal.asDouble()));
     }
   }

common/src/main/java/io/druid/math/expr/ExprEval.java

@@ -19,9 +19,7 @@
 
 package io.druid.math.expr;
 
-import com.google.common.base.Preconditions;
 import com.google.common.primitives.Doubles;
-import com.google.common.primitives.Ints;
 import io.druid.common.config.NullHandling;
 import io.druid.common.guava.GuavaUtils;
 import io.druid.java.util.common.IAE;
@@ -32,7 +30,7 @@
  */
 public abstract class ExprEval<T>
 {
-  public static ExprEval ofLong(Number longValue)
+  public static ExprEval ofLong(@Nullable Number longValue)
   {
     return new LongExprEval(longValue);
   }
@@ -42,7 +40,7 @@ public static ExprEval of(long longValue)
     return new LongExprEval(longValue);
   }
 
-  public static ExprEval ofDouble(Number doubleValue)
+  public static ExprEval ofDouble(@Nullable Number doubleValue)
   {
     return new DoubleExprEval(doubleValue);
   }
@@ -71,7 +69,7 @@ public static ExprEval of(boolean value, ExprType type)
     }
   }
 
-  public static ExprEval bestEffortOf(Object val)
+  public static ExprEval bestEffortOf(@Nullable Object val)
   {
     if (val instanceof ExprEval) {
       return (ExprEval) val;
@@ -85,6 +83,7 @@ public static ExprEval bestEffortOf(Object val)
     return new StringExprEval(val == null ? null : String.valueOf(val));
   }
 
+  @Nullable
   final T value;
 
   private ExprEval(T value)
@@ -99,10 +98,10 @@ public Object value()
     return value;
   }
 
-  public boolean isNull()
-  {
-    return value == null;
-  }
+  /**
+   * returns true if numeric primitive value for this ExprEval is null, otherwise false.
+   */
+  public abstract boolean isNumericNull();
 
   public abstract int asInt();
 
@@ -125,7 +124,7 @@ public String asString()
   private abstract static class NumericExprEval extends ExprEval<Number>
   {
 
-    private NumericExprEval(Number value)
+    private NumericExprEval(@Nullable Number value)
     {
       super(value);
     }
@@ -147,13 +146,19 @@ public final double asDouble()
     {
       return value.doubleValue();
     }
+
+    @Override
+    public boolean isNumericNull()
+    {
+      return value == null;
+    }
   }
 
   private static class DoubleExprEval extends NumericExprEval
   {
-    private DoubleExprEval(Number value)
+    private DoubleExprEval(@Nullable Number value)
     {
-      super(Preconditions.checkNotNull(value, "value"));
+      super(value == null ? NullHandling.defaultDoubleValue() : value);
     }
 
     @Override
@@ -175,7 +180,7 @@ public final ExprEval castTo(ExprType castTo)
         case DOUBLE:
           return this;
         case LONG:
-          return ExprEval.of(asLong());
+          return ExprEval.of(value == null ? null : asLong());
         case STRING:
           return ExprEval.of(asString());
       }
@@ -191,9 +196,9 @@ public Expr toExpr()
 
   private static class LongExprEval extends NumericExprEval
   {
-    private LongExprEval(Number value)
+    private LongExprEval(@Nullable Number value)
     {
-      super(Preconditions.checkNotNull(value, "value"));
+      super(value == null ? NullHandling.defaultLongValue() : value);
     }
 
     @Override
@@ -213,7 +218,7 @@ public final ExprEval castTo(ExprType castTo)
     {
       switch (castTo) {
         case DOUBLE:
-          return ExprEval.of(asDouble());
+          return ExprEval.of(value == null ? null : asDouble());
         case LONG:
           return this;
         case STRING:
@@ -231,6 +236,8 @@ public Expr toExpr()
 
   private static class StringExprEval extends ExprEval<String>
   {
+    private Number numericVal;
+
     private StringExprEval(@Nullable String value)
     {
       super(NullHandling.emptyToNullIfNeeded(value));
@@ -245,36 +252,63 @@ public final ExprType type()
     @Override
     public final int asInt()
     {
-      if (value == null) {
+      Number number = asNumber();
+      if (number == null) {
         assert NullHandling.replaceWithDefault();
         return 0;
       }
-
-      final Integer theInt = Ints.tryParse(value);
-      assert NullHandling.replaceWithDefault() || theInt != null;
-      return theInt == null ? 0 : theInt;
+      return number.intValue();
     }
 
     @Override
     public final long asLong()
     {
-      // GuavaUtils.tryParseLong handles nulls, no need for special null handling here.
-      final Long theLong = GuavaUtils.tryParseLong(value);
-      assert NullHandling.replaceWithDefault() || theLong != null;
-      return theLong == null ? 0L : theLong;
+      Number number = asNumber();
+      if (number == null) {
+        assert NullHandling.replaceWithDefault();
+        return 0L;
+      }
+      return number.longValue();
     }
 
     @Override
     public final double asDouble()
     {
-      if (value == null) {
+      Number number = asNumber();
+      if (number == null) {
         assert NullHandling.replaceWithDefault();
-        return 0.0;
+        return 0.0d;
+      }
+      return number.doubleValue();
+    }
+
+    @Nullable
+    private Number asNumber()
+    {
+      if (value == null) {
+        return null;
       }
+      if (numericVal != null) {
+        // Optimization for non-null case.
+        return numericVal;
+      }
+      Number rv;
+      Long v = GuavaUtils.tryParseLong(value);
+      // Do NOT use ternary operator here, because it makes Java to convert Long to Double
+      if (v != null) {
+        rv = v;
+      } else {
+        rv = Doubles.tryParse(value);
+      }
+
+      numericVal = rv;
+      return rv;
+    }
 
-      final Double theDouble = Doubles.tryParse(value);
-      assert NullHandling.replaceWithDefault() || theDouble != null;
-      return theDouble == null ? 0.0 : theDouble;
+    @Override
+    public boolean isNumericNull()
+    {
+      return asNumber() == null;
     }
 
     @Override
@@ -288,9 +322,9 @@ public final ExprEval castTo(ExprType castTo)
     {
       switch (castTo) {
         case DOUBLE:
-          return ExprEval.of(asDouble());
+          return ExprEval.ofDouble(asNumber());
         case LONG:
-          return ExprEval.of(asLong());
+          return ExprEval.ofLong(asNumber());
         case STRING:
           return this;
       }