Allow function implementation choices to have different dependencies

yaoqi · Jan 31, 2019 · 63df5ce · 63df5ce
1 parent 29b8355
commit 63df5ce
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 21 deletions.
diff --git a/presto-main/src/main/java/com/facebook/presto/operator/scalar/ArrayDistinctFromOperator.java b/presto-main/src/main/java/com/facebook/presto/operator/scalar/ArrayDistinctFromOperator.java
@@ -45,7 +45,6 @@ public static boolean isDistinctFrom(
                     returnType = StandardTypes.BOOLEAN,
                     argumentTypes = {"E", "E"},
                     convention = @Convention(arguments = {BLOCK_POSITION, BLOCK_POSITION}, result = FAIL_ON_NULL)) MethodHandle function,
-            @TypeParameter("array(E)") Type type,
             @SqlType("array(E)") Block left,
             @IsNull boolean leftNull,
             @SqlType("array(E)") Block right,
@@ -93,7 +92,6 @@ public static boolean isDistinctFrom(
     {
         return isDistinctFrom(
                 elementIsDistinctFrom,
-                type,
                 (Block) type.getObject(left, leftPosition),
                 left.isNull(leftPosition),
                 (Block) type.getObject(right, rightPosition),

diff --git a/.../java/com/facebook/presto/operator/scalar/annotations/ParametricScalarImplementation.java b/.../java/com/facebook/presto/operator/scalar/annotations/ParametricScalarImplementation.java
@@ -116,11 +116,6 @@ private ParametricScalarImplementation(
             checkArgument(specializedJavaType != Object.class, "specializedTypeParameter must not contain Object.class entries");
             checkArgument(!Primitives.isWrapperType(specializedJavaType), "specializedTypeParameter must not contain boxed primitive types");
         }
-        for (int i = 1; i < choices.size(); i++) {
-            checkCondition(Objects.equals(choices.get(i).checkDependencies(), choices.get(0).checkDependencies()), FUNCTION_IMPLEMENTATION_ERROR, "Implementations for the same function signature must have matching dependencies: %s", signature);
-            checkCondition(Objects.equals(choices.get(i).getConstructorDependencies(), choices.get(0).getConstructorDependencies()), FUNCTION_IMPLEMENTATION_ERROR, "Implementations for the same function signature must have matching constructor dependencies: %s", signature);
-            checkCondition(Objects.equals(choices.get(i).getConstructor(), choices.get(0).getConstructor()), FUNCTION_IMPLEMENTATION_ERROR, "Implementations for the same function signature must have matching constructors: %s", signature);
-        }
     }
 
     public Optional<ScalarFunctionImplementation> specialize(Signature boundSignature, BoundVariables boundVariables, TypeManager typeManager, FunctionRegistry functionRegistry, boolean isDeterministic)
@@ -199,17 +194,10 @@ List<Optional<Class<?>>> getArgumentNativeContainerTypes()
         return argumentNativeContainerTypes;
     }
 
-    public List<ImplementationDependency> getDependencies()
-    {
-        // All choices are required to have the same dependencies at this time. This is asserted in the constructor.
-        return choices.get(0).getDependencies();
-    }
-
     @VisibleForTesting
-    public List<ImplementationDependency> getConstructorDependencies()
+    public List<ParametricScalarImplementationChoice> getChoices()
     {
-        // All choices are required to have the same constructor dependencies at this time. This is asserted in the constructor.
-        return choices.get(0).getConstructorDependencies();
+        return choices;
     }
 
     Class<?> getReturnNativeContainerType()
@@ -370,6 +358,7 @@ public MethodHandle getMethodHandle()
             return methodHandle;
         }
 
+        @VisibleForTesting
         public List<ImplementationDependency> getDependencies()
         {
             return dependencies;
@@ -396,7 +385,7 @@ public boolean checkDependencies()
         }
 
         @VisibleForTesting
-        List<ImplementationDependency> getConstructorDependencies()
+        public List<ImplementationDependency> getConstructorDependencies()
         {
             return constructorDependencies;
         }

diff --git a/presto-main/src/test/java/com/facebook/presto/operator/TestAnnotationEngineForScalars.java b/presto-main/src/test/java/com/facebook/presto/operator/TestAnnotationEngineForScalars.java
@@ -22,6 +22,7 @@
 import com.facebook.presto.operator.annotations.TypeImplementationDependency;
 import com.facebook.presto.operator.scalar.ParametricScalar;
 import com.facebook.presto.operator.scalar.ScalarFunctionImplementation;
+import com.facebook.presto.operator.scalar.annotations.ParametricScalarImplementation.ParametricScalarImplementationChoice;
 import com.facebook.presto.operator.scalar.annotations.ScalarFromAnnotationsParser;
 import com.facebook.presto.spi.block.Block;
 import com.facebook.presto.spi.function.Description;
@@ -424,7 +425,9 @@ public void testSimpleInjectionScalarParse()
         assertEquals(functions.size(), 1);
         ParametricScalar scalar = (ParametricScalar) functions.get(0);
         assertImplementationCount(scalar, 0, 0, 1);
-        List<ImplementationDependency> dependencies = scalar.getImplementations().getGenericImplementations().get(0).getDependencies();
+        List<ParametricScalarImplementationChoice> parametricScalarImplementationChoices = scalar.getImplementations().getGenericImplementations().get(0).getChoices();
+        assertEquals(parametricScalarImplementationChoices.size(), 1);
+        List<ImplementationDependency> dependencies = parametricScalarImplementationChoices.get(0).getDependencies();
         assertEquals(dependencies.size(), 1);
         assertTrue(dependencies.get(0) instanceof LiteralImplementationDependency);
 
@@ -482,9 +485,11 @@ public void testConstructorInjectionScalarParse()
         assertEquals(functions.size(), 1);
         ParametricScalar scalar = (ParametricScalar) functions.get(0);
         assertImplementationCount(scalar, 2, 0, 1);
-        List<ImplementationDependency> dependencies = scalar.getImplementations().getGenericImplementations().get(0).getDependencies();
+        List<ParametricScalarImplementationChoice> parametricScalarImplementationChoices = scalar.getImplementations().getGenericImplementations().get(0).getChoices();
+        assertEquals(parametricScalarImplementationChoices.size(), 1);
+        List<ImplementationDependency> dependencies = parametricScalarImplementationChoices.get(0).getDependencies();
         assertEquals(dependencies.size(), 0);
-        List<ImplementationDependency> constructorDependencies = scalar.getImplementations().getGenericImplementations().get(0).getConstructorDependencies();
+        List<ImplementationDependency> constructorDependencies = parametricScalarImplementationChoices.get(0).getConstructorDependencies();
         assertEquals(constructorDependencies.size(), 1);
         assertTrue(constructorDependencies.get(0) instanceof TypeImplementationDependency);
 
@@ -561,7 +566,9 @@ public void testPartiallyFixedTypeParameterParse()
         assertEquals(functions.size(), 1);
         ParametricScalar scalar = (ParametricScalar) functions.get(0);
         assertImplementationCount(scalar, 0, 0, 1);
-        List<ImplementationDependency> dependencies = scalar.getImplementations().getGenericImplementations().get(0).getDependencies();
+        List<ParametricScalarImplementationChoice> parametricScalarImplementationChoices = scalar.getImplementations().getGenericImplementations().get(0).getChoices();
+        assertEquals(parametricScalarImplementationChoices.size(), 1);
+        List<ImplementationDependency> dependencies = parametricScalarImplementationChoices.get(0).getDependencies();
         assertEquals(dependencies.size(), 1);
 
         assertEquals(scalar.getSignature(), expectedSignature);