NPE thrown when empty/null is passes to TimeDimExtractionFn (#4601)

* NPE thrown when empty/null is passes to TimeDimExtractionFn

* Add @Nullable where ever applicable

* Add @Nullable to SearchQuerySpec.apply()

* Remove unused

processing/src/main/java/io/druid/query/extraction/BucketExtractionFn.java

@@ -56,8 +56,13 @@ public class BucketExtractionFn implements ExtractionFn
   }
 
   @Override
-  public String apply(Object value)
+  @Nullable
+  public String apply(@Nullable Object value)
   {
+    if (value == null) {
+      return null;
+    }
+
     if (value instanceof Number) {
       return bucket(((Number) value).doubleValue());
     } else if (value instanceof String) {
@@ -67,8 +72,13 @@ public class BucketExtractionFn implements ExtractionFn
   }
 
   @Override
-  public String apply(String value)
+  @Nullable
+  public String apply(@Nullable String value)
   {
+    if (value == null) {
+      return null;
+    }
+
     try {
       return bucket(Double.parseDouble(value));
     }

processing/src/main/java/io/druid/query/extraction/CascadeExtractionFn.java

@@ -25,6 +25,7 @@ import com.google.common.base.Joiner;
 import com.google.common.base.Preconditions;
 import com.google.common.primitives.Bytes;
 
+import javax.annotation.Nullable;
 import java.util.Arrays;
 
 public class CascadeExtractionFn implements ExtractionFn
@@ -40,14 +41,16 @@ public class CascadeExtractionFn implements ExtractionFn
           return new byte[0];
         }
 
+        @Nullable
         @Override
-        public String apply(Object value)
+        public String apply(@Nullable Object value)
         {
           return null;
         }
 
+        @Nullable
         @Override
-        public String apply(String value)
+        public String apply(@Nullable String value)
         {
           return null;
         }
@@ -113,13 +116,15 @@ public class CascadeExtractionFn implements ExtractionFn
   }
 
   @Override
-  public String apply(Object value)
+  @Nullable
+  public String apply(@Nullable Object value)
   {
     return chainedExtractionFn.apply(value);
   }
 
   @Override
-  public String apply(String value)
+  @Nullable
+  public String apply(@Nullable String value)
   {
     return chainedExtractionFn.apply(value);
   }
@@ -195,12 +200,14 @@ public class CascadeExtractionFn implements ExtractionFn
       return (child != null) ? Bytes.concat(fnCacheKey, child.getCacheKey()) : fnCacheKey;
     }
 
-    public String apply(Object value)
+    @Nullable
+    public String apply(@Nullable Object value)
     {
       return fn.apply((child != null) ? child.apply(value) : value);
     }
 
-    public String apply(String value)
+    @Nullable
+    public String apply(@Nullable String value)
     {
       return fn.apply((child != null) ? child.apply(value) : value);
     }

processing/src/main/java/io/druid/query/extraction/DimExtractionFn.java

@@ -19,12 +19,14 @@
 
 package io.druid.query.extraction;
 
+import javax.annotation.Nullable;
 import java.util.Objects;
 
 public abstract class DimExtractionFn implements ExtractionFn
 {
   @Override
-  public String apply(Object value)
+  @Nullable
+  public String apply(@Nullable Object value)
   {
     return apply(Objects.toString(value, null));
   }

processing/src/main/java/io/druid/query/extraction/ExtractionFn.java

@@ -24,6 +24,8 @@ import com.fasterxml.jackson.annotation.JsonTypeInfo;
 import io.druid.query.lookup.LookupExtractionFn;
 import io.druid.query.lookup.RegisteredLookupExtractionFn;
 
+import javax.annotation.Nullable;
+
 /**
  */
 @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "type")
@@ -75,7 +77,8 @@ public interface ExtractionFn
    *
    * @return a value that should be used instead of the original
    */
-  public String apply(Object value);
+  @Nullable
+  public String apply(@Nullable Object value);
 
   /**
    * The "extraction" function.  This should map a String value into some other String value.
@@ -87,7 +90,8 @@ public interface ExtractionFn
    *
    * @return a value that should be used instead of the original
    */
-  public String apply(String value);
+  @Nullable
+  public String apply(@Nullable String value);
 
   /**
    * The "extraction" function.  This should map a long value into some String value.

processing/src/main/java/io/druid/query/extraction/FunctionalExtraction.java

@@ -105,7 +105,8 @@ public abstract class FunctionalExtraction extends DimExtractionFn
   }
 
   @Override
-  public String apply(String value)
+  @Nullable
+  public String apply(@Nullable String value)
   {
     return extractionFunction.apply(value);
   }

processing/src/main/java/io/druid/query/extraction/IdentityExtractionFn.java

@@ -21,6 +21,8 @@ package io.druid.query.extraction;
 
 import com.google.common.base.Strings;
 
+import javax.annotation.Nullable;
+
 public class IdentityExtractionFn implements ExtractionFn
 {
   private static final IdentityExtractionFn instance = new IdentityExtractionFn();
@@ -37,13 +39,15 @@ public class IdentityExtractionFn implements ExtractionFn
   }
 
   @Override
-  public String apply(Object value)
+  @Nullable
+  public String apply(@Nullable Object value)
   {
     return value == null ? null : Strings.emptyToNull(value.toString());
   }
 
   @Override
-  public String apply(String value)
+  @Nullable
+  public String apply(@Nullable String value)
   {
     return Strings.emptyToNull(value);
   }

processing/src/main/java/io/druid/query/extraction/JavaScriptExtractionFn.java

@@ -32,6 +32,7 @@ import org.mozilla.javascript.Context;
 import org.mozilla.javascript.ContextFactory;
 import org.mozilla.javascript.ScriptableObject;
 
+import javax.annotation.Nullable;
 import java.nio.ByteBuffer;
 
 public class JavaScriptExtractionFn implements ExtractionFn
@@ -111,7 +112,8 @@ public class JavaScriptExtractionFn implements ExtractionFn
   }
 
   @Override
-  public String apply(Object value)
+  @Nullable
+  public String apply(@Nullable Object value)
   {
     if (fn == null) {
       throw new ISE("JavaScript is disabled");
@@ -121,7 +123,8 @@ public class JavaScriptExtractionFn implements ExtractionFn
   }
 
   @Override
-  public String apply(String value)
+  @Nullable
+  public String apply(@Nullable String value)
   {
     return this.apply((Object) Strings.emptyToNull(value));
   }

processing/src/main/java/io/druid/query/extraction/LowerExtractionFn.java

@@ -50,7 +50,7 @@ public class LowerExtractionFn extends DimExtractionFn
 
   @Nullable
   @Override
-  public String apply(String key)
+  public String apply(@Nullable String key)
   {
     if (Strings.isNullOrEmpty(key)) {
       return null;

processing/src/main/java/io/druid/query/extraction/MatchingDimExtractionFn.java

@@ -25,6 +25,7 @@ import com.google.common.base.Preconditions;
 import com.google.common.base.Strings;
 import io.druid.java.util.common.StringUtils;
 
+import javax.annotation.Nullable;
 import java.nio.ByteBuffer;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -57,8 +58,9 @@ public class MatchingDimExtractionFn extends DimExtractionFn
                      .array();
   }
 
+  @Nullable
   @Override
-  public String apply(String dimValue)
+  public String apply(@Nullable String dimValue)
   {
     if (Strings.isNullOrEmpty(dimValue)) {
       // We'd return null whether or not the pattern matched

processing/src/main/java/io/druid/query/extraction/RegexDimExtractionFn.java

@@ -26,6 +26,7 @@ import com.google.common.base.Strings;
 import com.google.common.primitives.Ints;
 import io.druid.java.util.common.StringUtils;
 
+import javax.annotation.Nullable;
 import java.nio.ByteBuffer;
 import java.util.Objects;
 import java.util.regex.Matcher;
@@ -100,8 +101,9 @@ public class RegexDimExtractionFn extends DimExtractionFn
                      .array();
   }
 
+  @Nullable
   @Override
-  public String apply(String dimValue)
+  public String apply(@Nullable String dimValue)
   {
     final String retVal;
     final Matcher matcher = pattern.matcher(Strings.nullToEmpty(dimValue));

processing/src/main/java/io/druid/query/extraction/SearchQuerySpecDimExtractionFn.java

@@ -25,6 +25,7 @@ import com.google.common.base.Preconditions;
 import com.google.common.base.Strings;
 import io.druid.query.search.search.SearchQuerySpec;
 
+import javax.annotation.Nullable;
 import java.nio.ByteBuffer;
 
 /**
@@ -59,8 +60,9 @@ public class SearchQuerySpecDimExtractionFn extends DimExtractionFn
                      .array();
   }
 
+  @Nullable
   @Override
-  public String apply(String dimValue)
+  public String apply(@Nullable String dimValue)
   {
     return searchQuerySpec.accept(dimValue) ? Strings.emptyToNull(dimValue) : null;
   }

processing/src/main/java/io/druid/query/extraction/StringFormatExtractionFn.java

@@ -26,6 +26,7 @@ import com.google.common.base.Preconditions;
 import com.google.common.base.Strings;
 import io.druid.java.util.common.StringUtils;
 
+import javax.annotation.Nullable;
 import java.nio.ByteBuffer;
 
 /**
@@ -94,8 +95,9 @@ public class StringFormatExtractionFn extends DimExtractionFn
                      .array();
   }
 
+  @Nullable
   @Override
-  public String apply(String value)
+  public String apply(@Nullable String value)
   {
     if (value == null) {
       if (nullHandling == NullHandling.RETURNNULL) {

processing/src/main/java/io/druid/query/extraction/StrlenExtractionFn.java

@@ -21,6 +21,8 @@ package io.druid.query.extraction;
 
 import com.fasterxml.jackson.annotation.JsonCreator;
 
+import javax.annotation.Nullable;
+
 public class StrlenExtractionFn extends DimExtractionFn
 {
   private static final StrlenExtractionFn INSTANCE = new StrlenExtractionFn();
@@ -36,7 +38,7 @@ public class StrlenExtractionFn extends DimExtractionFn
   }
 
   @Override
-  public String apply(String value)
+  public String apply(@Nullable String value)
   {
     return String.valueOf(value == null ? 0 : value.length());
   }

processing/src/main/java/io/druid/query/extraction/SubstringDimExtractionFn.java

@@ -61,8 +61,9 @@ public class SubstringDimExtractionFn extends DimExtractionFn
                      .array();
   }
 
+  @Nullable
   @Override
-  public String apply(String dimValue)
+  public String apply(@Nullable String dimValue)
   {
     if (Strings.isNullOrEmpty(dimValue)) {
       return null;

processing/src/main/java/io/druid/query/extraction/TimeDimExtractionFn.java

@@ -22,9 +22,11 @@ package io.druid.query.extraction;
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.google.common.base.Preconditions;
+import com.google.common.base.Strings;
 import com.ibm.icu.text.SimpleDateFormat;
 import io.druid.java.util.common.StringUtils;
 
+import javax.annotation.Nullable;
 import java.nio.ByteBuffer;
 import java.text.ParseException;
 import java.util.Date;
@@ -71,9 +73,14 @@ public class TimeDimExtractionFn extends DimExtractionFn
                      .array();
   }
 
+  @Nullable
   @Override
-  public String apply(String dimValue)
+  public String apply(@Nullable String dimValue)
   {
+    if (Strings.isNullOrEmpty(dimValue)) {
+      return null;
+    }
+
     Date date;
     try {
       date = timeFormatter.get().parse(dimValue);

processing/src/main/java/io/druid/query/extraction/TimeFormatExtractionFn.java

@@ -31,6 +31,7 @@ import org.joda.time.format.DateTimeFormat;
 import org.joda.time.format.DateTimeFormatter;
 import org.joda.time.format.ISODateTimeFormat;
 
+import javax.annotation.Nullable;
 import java.nio.ByteBuffer;
 import java.util.Locale;
 
@@ -128,8 +129,13 @@ public class TimeFormatExtractionFn implements ExtractionFn
   }
 
   @Override
-  public String apply(Object value)
+  @Nullable
+  public String apply(@Nullable Object value)
   {
+    if (value == null) {
+      return null;
+    }
+
     if (asMillis && value instanceof String) {
       final Long theLong = GuavaUtils.tryParseLong((String) value);
       return theLong == null ? apply(new DateTime(value).getMillis()) : apply(theLong.longValue());
@@ -139,7 +145,8 @@ public class TimeFormatExtractionFn implements ExtractionFn
   }
 
   @Override
-  public String apply(String value)
+  @Nullable
+  public String apply(@Nullable String value)
   {
     return apply((Object) value);
   }

processing/src/main/java/io/druid/query/extraction/UpperExtractionFn.java

@@ -49,7 +49,7 @@ public class UpperExtractionFn extends DimExtractionFn
    */
   @Nullable
   @Override
-  public String apply(String key)
+  public String apply(@Nullable String key)
   {
     if (Strings.isNullOrEmpty(key)) {
       return null;

processing/src/main/java/io/druid/query/lookup/RegisteredLookupExtractionFn.java

@@ -105,13 +105,15 @@ public class RegisteredLookupExtractionFn implements ExtractionFn
   }
 
   @Override
-  public String apply(Object value)
+  @Nullable
+  public String apply(@Nullable Object value)
   {
     return ensureDelegate().apply(value);
   }
 
   @Override
-  public String apply(String value)
+  @Nullable
+  public String apply(@Nullable String value)
   {
     return ensureDelegate().apply(value);
   }

processing/src/main/java/io/druid/query/search/search/AllSearchQuerySpec.java

@@ -19,6 +19,8 @@
 
 package io.druid.query.search.search;
 
+import javax.annotation.Nullable;
+
 /**
  */
 public class AllSearchQuerySpec implements SearchQuerySpec
@@ -26,7 +28,7 @@ public class AllSearchQuerySpec implements SearchQuerySpec
   private static final byte CACHE_TYPE_ID = 0x7f;
 
   @Override
-  public boolean accept(String dimVal)
+  public boolean accept(@Nullable String dimVal)
   {
     return true;
   }

processing/src/main/java/io/druid/query/search/search/ContainsSearchQuerySpec.java

@@ -24,6 +24,7 @@ import com.fasterxml.jackson.annotation.JsonProperty;
 import com.google.common.base.Objects;
 import io.druid.java.util.common.StringUtils;
 
+import javax.annotation.Nullable;
 import java.nio.ByteBuffer;
 
 /**
@@ -59,7 +60,7 @@ public class ContainsSearchQuerySpec implements SearchQuerySpec
   }
 
   @Override
-  public boolean accept(String dimVal)
+  public boolean accept(@Nullable String dimVal)
   {
     if (dimVal == null || value == null) {
       return false;

processing/src/main/java/io/druid/query/search/search/FragmentSearchQuerySpec.java

@@ -23,6 +23,7 @@ import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
 import io.druid.java.util.common.StringUtils;
 
+import javax.annotation.Nullable;
 import java.nio.ByteBuffer;
 import java.util.Arrays;
 import java.util.List;
@@ -77,7 +78,7 @@ public class FragmentSearchQuerySpec implements SearchQuerySpec
   }
 
   @Override
-  public boolean accept(String dimVal)
+  public boolean accept(@Nullable String dimVal)
   {
     if (dimVal == null || values == null) {
       return false;

processing/src/main/java/io/druid/query/search/search/RegexSearchQuerySpec.java

@@ -24,6 +24,7 @@ import com.fasterxml.jackson.annotation.JsonProperty;
 import com.google.common.base.Preconditions;
 import io.druid.java.util.common.StringUtils;
 
+import javax.annotation.Nullable;
 import java.nio.ByteBuffer;
 import java.util.regex.Pattern;
 
@@ -74,7 +75,7 @@ public class RegexSearchQuerySpec implements SearchQuerySpec
   }
 
   @Override
-  public boolean accept(String dimVal)
+  public boolean accept(@Nullable String dimVal)
   {
     if (dimVal == null) {
       return false;

processing/src/main/java/io/druid/query/search/search/SearchQuerySpec.java

@@ -22,6 +22,8 @@ package io.druid.query.search.search;
 import com.fasterxml.jackson.annotation.JsonSubTypes;
 import com.fasterxml.jackson.annotation.JsonTypeInfo;
 
+import javax.annotation.Nullable;
+
 /**
  */
 @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "type")
@@ -34,7 +36,7 @@ import com.fasterxml.jackson.annotation.JsonTypeInfo;
 })
 public interface SearchQuerySpec
 {
-  public boolean accept(String dimVal);
+  public boolean accept(@Nullable String dimVal);
 
   public byte[] getCacheKey();
 }

processing/src/test/java/io/druid/query/extraction/TimeDimExtractionFnTest.java

@@ -40,6 +40,16 @@ public class TimeDimExtractionFnTest
       "12/21/2012"
   };
 
+  @Test
+  public void testEmptyAndNullExtraction()
+  {
+    Set<String> testPeriod = Sets.newHashSet();
+    ExtractionFn extractionFn = new TimeDimExtractionFn("MM/dd/yyyy", "MM/yyyy");
+
+    Assert.assertNull(extractionFn.apply(null));
+    Assert.assertNull(extractionFn.apply(""));
+  }
+
   @Test
   public void testMonthExtraction()
   {

processing/src/test/java/io/druid/segment/filter/SelectorFilterTest.java

@@ -30,6 +30,7 @@ import io.druid.data.input.impl.TimeAndDimsParseSpec;
 import io.druid.data.input.impl.TimestampSpec;
 import io.druid.java.util.common.Pair;
 import io.druid.query.extraction.MapLookupExtractor;
+import io.druid.query.extraction.TimeDimExtractionFn;
 import io.druid.query.filter.ExtractionDimFilter;
 import io.druid.query.filter.InDimFilter;
 import io.druid.query.filter.SelectorDimFilter;
@@ -58,7 +59,7 @@ public class SelectorFilterTest extends BaseFilterTest
       new TimeAndDimsParseSpec(
           new TimestampSpec(TIMESTAMP_COLUMN, "iso", new DateTime("2000")),
           new DimensionsSpec(
-              DimensionsSpec.getDefaultSchemas(ImmutableList.of("dim0", "dim1", "dim2", "dim3")),
+              DimensionsSpec.getDefaultSchemas(ImmutableList.of("dim0", "dim1", "dim2", "dim3", "dim6")),
               null,
               null
           )
@@ -66,9 +67,9 @@ public class SelectorFilterTest extends BaseFilterTest
   );
 
   private static final List<InputRow> ROWS = ImmutableList.of(
-      PARSER.parse(ImmutableMap.<String, Object>of("dim0", "0", "dim1", "", "dim2", ImmutableList.of("a", "b"))),
-      PARSER.parse(ImmutableMap.<String, Object>of("dim0", "1", "dim1", "10", "dim2", ImmutableList.of())),
-      PARSER.parse(ImmutableMap.<String, Object>of("dim0", "2", "dim1", "2", "dim2", ImmutableList.of(""))),
+      PARSER.parse(ImmutableMap.<String, Object>of("dim0", "0", "dim1", "", "dim2", ImmutableList.of("a", "b"), "dim6", "2017-07-25")),
+      PARSER.parse(ImmutableMap.<String, Object>of("dim0", "1", "dim1", "10", "dim2", ImmutableList.of(), "dim6", "2017-07-25")),
+      PARSER.parse(ImmutableMap.<String, Object>of("dim0", "2", "dim1", "2", "dim2", ImmutableList.of(""), "dim6", "2017-05-25")),
       PARSER.parse(ImmutableMap.<String, Object>of("dim0", "3", "dim1", "1", "dim2", ImmutableList.of("a"))),
       PARSER.parse(ImmutableMap.<String, Object>of("dim0", "4", "dim1", "def", "dim2", ImmutableList.of("c"))),
       PARSER.parse(ImmutableMap.<String, Object>of("dim0", "5", "dim1", "abc"))
@@ -91,6 +92,15 @@ public class SelectorFilterTest extends BaseFilterTest
     BaseFilterTest.tearDown(SelectorFilterTest.class.getName());
   }
 
+  @Test
+  public void testWithTimeExtractionFnNull()
+  {
+    assertFilterMatches(new SelectorDimFilter("dim0", null, new TimeDimExtractionFn("yyyy-mm-dd", "yyyy-mm")), ImmutableList.<String>of());
+    assertFilterMatches(new SelectorDimFilter("dim6", null, new TimeDimExtractionFn("yyyy-mm-dd", "yyyy-mm")), ImmutableList.<String>of("3", "4", "5"));
+    assertFilterMatches(new SelectorDimFilter("dim6", "2017-07", new TimeDimExtractionFn("yyyy-mm-dd", "yyyy-mm")), ImmutableList.<String>of("0", "1"));
+    assertFilterMatches(new SelectorDimFilter("dim6", "2017-05", new TimeDimExtractionFn("yyyy-mm-dd", "yyyy-mm")), ImmutableList.<String>of("2"));
+  }
+
   @Test
   public void testSingleValueStringColumnWithoutNulls()
   {