DRILL-2014: Add mechanism to detect and prevent registering Drill fun…

…ctions with matching signature. Eliminate existing duplicate functions.
apache · Jan 19, 2015 · 952114f · 952114f
1 parent a418af1
commit 952114f
Show file tree

Hide file tree

Showing 7 changed files with 35 additions and 12 deletions.
diff --git a/exec/java-exec/src/main/codegen/templates/NullOperator.java b/exec/java-exec/src/main/codegen/templates/NullOperator.java
@@ -35,7 +35,7 @@
 
 public class ${className} {
 
-  @FunctionTemplate(names = {"isNull", "isnull", "is null"}, scope = FunctionTemplate.FunctionScope.SIMPLE, nulls = FunctionTemplate.NullHandling.INTERNAL)
+  @FunctionTemplate(names = {"isnull", "is null"}, scope = FunctionTemplate.FunctionScope.SIMPLE, nulls = FunctionTemplate.NullHandling.INTERNAL)
   public static class IsNull implements DrillSimpleFunc {
 
     @Param ${mode.prefix}${minor.class}Holder input;
@@ -52,7 +52,7 @@ public void eval() {
     }
   }
 
-  @FunctionTemplate(names = {"isNotNull", "isnotnull", "is not null"}, scope = FunctionTemplate.FunctionScope.SIMPLE, nulls = FunctionTemplate.NullHandling.INTERNAL)
+  @FunctionTemplate(names = {"isnotnull", "is not null"}, scope = FunctionTemplate.FunctionScope.SIMPLE, nulls = FunctionTemplate.NullHandling.INTERNAL)
   public static class IsNotNull implements DrillSimpleFunc {
 
     @Param ${mode.prefix}${minor.class}Holder input;

diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFunctionRegistry.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFunctionRegistry.java
@@ -18,6 +18,7 @@
 package org.apache.drill.exec.expr.fn;
 
 import java.util.Collection;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map.Entry;
 import java.util.Set;
@@ -40,6 +41,12 @@ public class DrillFunctionRegistry {
 
   private ArrayListMultimap<String, DrillFuncHolder> methods = ArrayListMultimap.create();
 
+  /* Hash map to prevent registering functions with exactly matching signatures
+   * key: Function Name + Input's Major Type
+   * Value: Class name where function is implemented
+   */
+  private HashMap<String, String> functionSignatureMap = new HashMap<>();
+
   public DrillFunctionRegistry(DrillConfig config) {
     FunctionConverter converter = new FunctionConverter();
     Set<Class<? extends DrillFunc>> providerClasses = PathScanner.scanForImplementations(DrillFunc.class, config.getStringList(ExecConstants.FUNCTION_PACKAGES));
@@ -48,8 +55,24 @@ public DrillFunctionRegistry(DrillConfig config) {
       if (holder != null) {
         // register handle for each name the function can be referred to
         String[] names = holder.getRegisteredNames();
+
+        // Create the string for input types
+        String functionInput = "";
+        for (DrillFuncHolder.ValueReference ref : holder.parameters) {
+          functionInput += ref.getType().toString();
+        }
         for (String name : names) {
-          methods.put(name.toLowerCase(), holder);
+          String functionName = name.toLowerCase();
+          methods.put(functionName, holder);
+          String functionSignature = functionName + functionInput;
+
+          String existingImplementation;
+          if ((existingImplementation = functionSignatureMap.get(functionSignature)) != null) {
+            throw new AssertionError(String.format("Conflicting functions with similar signature found. Func Name: %s, Class name: %s " +
+                " Class name: %s", functionName, clazz.getName(), existingImplementation));
+          } else {
+            functionSignatureMap.put(functionSignature, clazz.getName());
+          }
         }
       } else {
         logger.warn("Unable to initialize function for class {}", clazz.getName());

diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/CastBigIntTimeStamp.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/CastBigIntTimeStamp.java
@@ -27,7 +27,7 @@
 import org.apache.drill.exec.record.RecordBatch;
 
 @SuppressWarnings("unused")
-@FunctionTemplate(names = {"castTIMESTAMP", "to_timestamp"}, scope = FunctionTemplate.FunctionScope.SIMPLE, nulls= NullHandling.NULL_IF_NULL)
+@FunctionTemplate(name = "castTIMESTAMP", scope = FunctionTemplate.FunctionScope.SIMPLE, nulls= NullHandling.NULL_IF_NULL)
 public class CastBigIntTimeStamp implements DrillSimpleFunc {
 
   @Param

diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/IsFalse.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/IsFalse.java
@@ -28,7 +28,7 @@
 
 public class IsFalse {
 
-  @FunctionTemplate(names = {"isFalse", "isfalse", "is false"}, scope = FunctionTemplate.FunctionScope.SIMPLE, nulls = FunctionTemplate.NullHandling.INTERNAL)
+  @FunctionTemplate(names = {"isfalse", "is false"}, scope = FunctionTemplate.FunctionScope.SIMPLE, nulls = FunctionTemplate.NullHandling.INTERNAL)
   public static class Optional implements DrillSimpleFunc {
 
     @Param NullableBitHolder in;
@@ -45,7 +45,7 @@ public void eval() {
     }
   }
 
-  @FunctionTemplate(names = {"isFalse", "isfalse", "is false"}, scope = FunctionTemplate.FunctionScope.SIMPLE, nulls = FunctionTemplate.NullHandling.INTERNAL)
+  @FunctionTemplate(names = {"isfalse", "is false"}, scope = FunctionTemplate.FunctionScope.SIMPLE, nulls = FunctionTemplate.NullHandling.INTERNAL)
   public static class Required implements DrillSimpleFunc {
 
     @Param BitHolder in;

diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/IsNotFalse.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/IsNotFalse.java
@@ -28,7 +28,7 @@
 
 public class IsNotFalse {
 
-  @FunctionTemplate(names = {"isNotFalse", "isnotfalse", "is not false"}, scope = FunctionTemplate.FunctionScope.SIMPLE, nulls = FunctionTemplate.NullHandling.INTERNAL)
+  @FunctionTemplate(names = {"isnotfalse", "is not false"}, scope = FunctionTemplate.FunctionScope.SIMPLE, nulls = FunctionTemplate.NullHandling.INTERNAL)
   public static class Optional implements DrillSimpleFunc {
 
     @Param NullableBitHolder in;
@@ -45,7 +45,7 @@ public void eval() {
     }
   }
 
-  @FunctionTemplate(names = {"isNotFalse", "isnotfalse", "is not false"}, scope = FunctionTemplate.FunctionScope.SIMPLE, nulls = FunctionTemplate.NullHandling.INTERNAL)
+  @FunctionTemplate(names = {"isnotfalse", "is not false"}, scope = FunctionTemplate.FunctionScope.SIMPLE, nulls = FunctionTemplate.NullHandling.INTERNAL)
   public static class Required implements DrillSimpleFunc {
 
     @Param BitHolder in;

diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/IsNotTrue.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/IsNotTrue.java
@@ -28,7 +28,7 @@
 
 public class IsNotTrue {
 
-  @FunctionTemplate(names = {"isNotTrue", "isnottrue", "is not true"}, scope = FunctionTemplate.FunctionScope.SIMPLE, nulls = FunctionTemplate.NullHandling.INTERNAL)
+  @FunctionTemplate(names = {"isnottrue", "is not true"}, scope = FunctionTemplate.FunctionScope.SIMPLE, nulls = FunctionTemplate.NullHandling.INTERNAL)
   public static class Optional implements DrillSimpleFunc {
 
     @Param NullableBitHolder in;
@@ -45,7 +45,7 @@ public void eval() {
     }
   }
 
-  @FunctionTemplate(names = {"isNotTrue", "isnottrue", "is not true"}, scope = FunctionTemplate.FunctionScope.SIMPLE, nulls = FunctionTemplate.NullHandling.INTERNAL)
+  @FunctionTemplate(names = {"isnottrue", "is not true"}, scope = FunctionTemplate.FunctionScope.SIMPLE, nulls = FunctionTemplate.NullHandling.INTERNAL)
   public static class Required implements DrillSimpleFunc {
 
     @Param BitHolder in;

diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/IsTrue.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/IsTrue.java
@@ -28,7 +28,7 @@
 
 public class IsTrue {
 
-  @FunctionTemplate(names = {"isTrue", "istrue", "is true"}, scope = FunctionTemplate.FunctionScope.SIMPLE, nulls = FunctionTemplate.NullHandling.INTERNAL)
+  @FunctionTemplate(names = {"istrue", "is true"}, scope = FunctionTemplate.FunctionScope.SIMPLE, nulls = FunctionTemplate.NullHandling.INTERNAL)
   public static class Optional implements DrillSimpleFunc {
 
     @Param NullableBitHolder in;
@@ -45,7 +45,7 @@ public void eval() {
     }
   }
 
-  @FunctionTemplate(names = {"isTrue", "istrue", "is true"}, scope = FunctionTemplate.FunctionScope.SIMPLE, nulls = FunctionTemplate.NullHandling.INTERNAL)
+  @FunctionTemplate(names = {"istrue", "is true"}, scope = FunctionTemplate.FunctionScope.SIMPLE, nulls = FunctionTemplate.NullHandling.INTERNAL)
   public static class Required implements DrillSimpleFunc {
 
     @Param BitHolder in;