From e060a9f283d716fdfb380e1b16282399d8e3b0a4 Mon Sep 17 00:00:00 2001
From: Gian Merlino <gianmerlino@gmail.com>
Date: Fri, 1 Apr 2016 18:35:24 -0700
Subject: [PATCH] Additional ExtractionFn null-handling adjustments.

Followup to comments on #2771.
---
 .../query/extraction/MatchingDimExtractionFn.java |  9 +++++++--
 .../extraction/JavaScriptExtractionFnTest.java    | 14 ++++++++++++++
 .../extraction/RegexDimExtractionFnTest.java      | 15 ++++++++++++++-
 3 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/processing/src/main/java/io/druid/query/extraction/MatchingDimExtractionFn.java b/processing/src/main/java/io/druid/query/extraction/MatchingDimExtractionFn.java
index ae2b6ddb9e3..9e5d16ec8f7 100644
--- a/processing/src/main/java/io/druid/query/extraction/MatchingDimExtractionFn.java
+++ b/processing/src/main/java/io/druid/query/extraction/MatchingDimExtractionFn.java
@@ -60,8 +60,13 @@ public class MatchingDimExtractionFn extends DimExtractionFn
   @Override
   public String apply(String dimValue)
   {
-    Matcher matcher = pattern.matcher(Strings.nullToEmpty(dimValue));
-    return matcher.find() ? Strings.emptyToNull(dimValue) : null;
+    if (Strings.isNullOrEmpty(dimValue)) {
+      // We'd return null whether or not the pattern matched
+      return null;
+    }
+
+    Matcher matcher = pattern.matcher(dimValue);
+    return matcher.find() ? dimValue : null;
   }
 
   @JsonProperty("expr")
diff --git a/processing/src/test/java/io/druid/query/extraction/JavaScriptExtractionFnTest.java b/processing/src/test/java/io/druid/query/extraction/JavaScriptExtractionFnTest.java
index f861f97ce19..0a9b86b020f 100644
--- a/processing/src/test/java/io/druid/query/extraction/JavaScriptExtractionFnTest.java
+++ b/processing/src/test/java/io/druid/query/extraction/JavaScriptExtractionFnTest.java
@@ -104,6 +104,20 @@ public class JavaScriptExtractionFnTest
     }
   }
 
+  @Test
+  public void testJavascriptIsNull()
+  {
+    String function = "function(x) { if (x == null) { return 'yes'; } else { return 'no' } }";
+    ExtractionFn extractionFn = new JavaScriptExtractionFn(function, false);
+
+    Assert.assertEquals("yes", extractionFn.apply((String) null));
+    Assert.assertEquals("yes", extractionFn.apply((Object) null));
+    Assert.assertEquals("yes", extractionFn.apply(""));
+    Assert.assertEquals("no", extractionFn.apply("abc"));
+    Assert.assertEquals("no", extractionFn.apply(new Object()));
+    Assert.assertEquals("no", extractionFn.apply(1));
+  }
+
   @Test
   public void testJavaScriptPorterStemmer()
   {
diff --git a/processing/src/test/java/io/druid/query/extraction/RegexDimExtractionFnTest.java b/processing/src/test/java/io/druid/query/extraction/RegexDimExtractionFnTest.java
index 080501ea061..480d1b6d991 100644
--- a/processing/src/test/java/io/druid/query/extraction/RegexDimExtractionFnTest.java
+++ b/processing/src/test/java/io/druid/query/extraction/RegexDimExtractionFnTest.java
@@ -116,12 +116,24 @@ public class RegexDimExtractionFnTest
   }
 
   @Test
-  public void testMissingValueReplacementFromNullAndEmpty()
+  public void testMissingValueReplacementWhenPatternDoesNotMatchNull()
   {
     String regex = "(bob)";
     ExtractionFn extractionFn = new RegexDimExtractionFn(regex, true, "NO MATCH");
     Assert.assertEquals("NO MATCH", extractionFn.apply(""));
     Assert.assertEquals("NO MATCH", extractionFn.apply(null));
+    Assert.assertEquals("NO MATCH", extractionFn.apply("abc"));
+    Assert.assertEquals("bob", extractionFn.apply("bob"));
+  }
+
+  @Test
+  public void testMissingValueReplacementWhenPatternMatchesNull()
+  {
+    String regex = "^()$";
+    ExtractionFn extractionFn = new RegexDimExtractionFn(regex, true, "NO MATCH");
+    Assert.assertEquals(null, extractionFn.apply(""));
+    Assert.assertEquals(null, extractionFn.apply(null));
+    Assert.assertEquals("NO MATCH", extractionFn.apply("abc"));
   }
 
   @Test
@@ -133,6 +145,7 @@ public class RegexDimExtractionFnTest
     Assert.assertEquals(null, extractionFn.apply(""));
     Assert.assertEquals(null, extractionFn.apply("abc"));
     Assert.assertEquals(null, extractionFn.apply("123"));
+    Assert.assertEquals("bob", extractionFn.apply("bobby"));
   }
 
   @Test