Support transaction in FunctionNamespaceManager

pom.xml

@@ -138,6 +138,7 @@
         <module>presto-proxy</module>
         <module>presto-kudu</module>
         <module>presto-elasticsearch</module>
+        <module>presto-sql-function</module>
     </modules>
 
     <dependencyManagement>
@@ -424,6 +425,12 @@
                 <version>${project.version}</version>
             </dependency>
 
+            <dependency>
+                <groupId>com.facebook.presto</groupId>
+                <artifactId>presto-sql-function</artifactId>
+                <version>${project.version}</version>
+            </dependency>
+
             <dependency>
                 <groupId>com.fasterxml.jackson.module</groupId>
                 <artifactId>jackson-module-afterburner</artifactId>

presto-geospatial/src/test/java/com/facebook/presto/plugin/geospatial/TestBingTileFunctions.java

@@ -60,8 +60,8 @@ protected void registerFunctions()
         for (Type type : plugin.getTypes()) {
             functionAssertions.getTypeRegistry().addType(type);
         }
-        functionAssertions.getMetadata().addFunctions(extractFunctions(plugin.getFunctions()));
-        functionAssertions.getMetadata().addFunctions(ImmutableList.of(APPLY_FUNCTION));
+        functionAssertions.getMetadata().registerBuiltInFunctions(extractFunctions(plugin.getFunctions()));
+        functionAssertions.getMetadata().registerBuiltInFunctions(ImmutableList.of(APPLY_FUNCTION));
         FunctionManager functionManager = functionAssertions.getMetadata().getFunctionManager();
         approxDistinct = functionManager.getAggregateFunctionImplementation(
                 functionManager.lookupFunction("approx_distinct", fromTypes(BING_TILE)));

presto-geospatial/src/test/java/com/facebook/presto/plugin/geospatial/TestSphericalGeoFunctions.java

@@ -47,7 +47,7 @@ protected void registerFunctions()
         for (Type type : plugin.getTypes()) {
             functionAssertions.getTypeRegistry().addType(type);
         }
-        functionAssertions.getMetadata().addFunctions(extractFunctions(plugin.getFunctions()));
+        functionAssertions.getMetadata().registerBuiltInFunctions(extractFunctions(plugin.getFunctions()));
     }
 
     @Test

presto-geospatial/src/test/java/com/facebook/presto/plugin/geospatial/aggregation/AbstractTestGeoAggregationFunctions.java

@@ -48,7 +48,7 @@ public void registerFunctions()
         for (Type type : plugin.getTypes()) {
             functionAssertions.getTypeRegistry().addType(type);
         }
-        functionAssertions.getMetadata().addFunctions(extractFunctions(plugin.getFunctions()));
+        functionAssertions.getMetadata().registerBuiltInFunctions(extractFunctions(plugin.getFunctions()));
         FunctionManager functionManager = functionAssertions.getMetadata().getFunctionManager();
         function = functionManager.getAggregateFunctionImplementation(
                 functionManager.lookupFunction(getFunctionName(), fromTypes(GEOMETRY)));

presto-main/pom.xml

@@ -71,6 +71,11 @@
             <artifactId>presto-memory-context</artifactId>
         </dependency>
 
+        <dependency>
+            <groupId>com.facebook.presto</groupId>
+            <artifactId>presto-sql-function</artifactId>
+        </dependency>
+
         <dependency>
             <groupId>com.google.code.findbugs</groupId>
             <artifactId>jsr305</artifactId>

presto-main/src/main/java/com/facebook/presto/metadata/BuiltInFunction.java

@@ -0,0 +1,26 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.facebook.presto.metadata;
+
+import com.facebook.presto.spi.function.SqlFunction;
+
+public abstract class BuiltInFunction
+        implements SqlFunction
+{
+    @Override
+    public boolean isCalledOnNullInput()
+    {
+        return false;
+    }
+}

presto-main/src/main/java/com/facebook/presto/metadata/BuiltInFunctionNamespaceManager.java

@@ -151,12 +151,12 @@
 import com.facebook.presto.operator.window.SqlWindowFunction;
 import com.facebook.presto.operator.window.WindowFunctionSupplier;
 import com.facebook.presto.spi.PrestoException;
-import com.facebook.presto.spi.QueryId;
 import com.facebook.presto.spi.block.Block;
 import com.facebook.presto.spi.block.BlockEncodingSerde;
 import com.facebook.presto.spi.function.FunctionHandle;
 import com.facebook.presto.spi.function.FunctionMetadata;
 import com.facebook.presto.spi.function.FunctionNamespaceManager;
+import com.facebook.presto.spi.function.FunctionNamespaceTransactionHandle;
 import com.facebook.presto.spi.function.OperatorType;
 import com.facebook.presto.spi.function.Signature;
 import com.facebook.presto.spi.function.SqlFunction;
@@ -349,9 +349,10 @@
 
 @ThreadSafe
 public class BuiltInFunctionNamespaceManager
-        implements FunctionNamespaceManager
+        implements FunctionNamespaceManager<BuiltInFunction>
 {
     public static final FullyQualifiedName.Prefix DEFAULT_NAMESPACE = FullyQualifiedName.of("presto.default.foo").getPrefix();
+    public static final String NAME = "_builtin";
 
     private final TypeManager typeManager;
     private final LoadingCache<Signature, SpecializedFunctionKey> specializedFunctionKeyCache;
@@ -664,11 +665,10 @@ public BuiltInFunctionNamespaceManager(
             builder.scalar(LegacyLogFunction.class);
         }
 
-        addFunctions(builder.getFunctions());
+        registerBuiltInFunctions(builder.getFunctions());
     }
 
-    @Override
-    public final synchronized void addFunctions(List<? extends SqlFunction> functions)
+    public synchronized void registerBuiltInFunctions(List<? extends BuiltInFunction> functions)
     {
         for (SqlFunction function : functions) {
             for (SqlFunction existingFunction : this.functions.list()) {
@@ -679,19 +679,46 @@ public final synchronized void addFunctions(List<? extends SqlFunction> function
     }
 
     @Override
-    public List<SqlFunction> listFunctions()
+    public void createFunction(BuiltInFunction function, boolean replace)
+    {
+        throw new UnsupportedOperationException("createFunction cannot be called on BuiltInFunctionNamespaceManager");
+    }
+
+    public String getName()
+    {
+        return NAME;
+    }
+
+    @Override
+    public FunctionNamespaceTransactionHandle beginTransaction()
+    {
+        return new EmptyTransactionHandle();
+    }
+
+    @Override
+    public void commit(FunctionNamespaceTransactionHandle transactionHandle)
+    {
+    }
+
+    @Override
+    public void rollback(FunctionNamespaceTransactionHandle transactionHandle)
+    {
+    }
+
+    @Override
+    public Collection<BuiltInFunction> listFunctions()
     {
         return functions.list();
     }
 
     @Override
-    public Collection<SqlFunction> getCandidates(QueryId queryId, FullyQualifiedName name)
+    public Collection<BuiltInFunction> getFunctions(Optional<? extends FunctionNamespaceTransactionHandle> transactionHandle, FullyQualifiedName functionName)
     {
-        return functions.get(name);
+        return functions.get(functionName);
     }
 
     @Override
-    public FunctionHandle getFunctionHandle(QueryId queryId, Signature signature)
+    public FunctionHandle getFunctionHandle(Optional<? extends FunctionNamespaceTransactionHandle> transactionHandle, Signature signature)
     {
         return new BuiltInFunctionHandle(signature);
     }
@@ -709,7 +736,7 @@ public FunctionMetadata getFunctionMetadata(FunctionHandle functionHandle)
             throwIfInstanceOf(e.getCause(), PrestoException.class);
             throw e;
         }
-        SqlFunction function = functionKey.getFunction();
+        BuiltInFunction function = functionKey.getFunction();
         Optional<OperatorType> operatorType = tryGetOperatorType(signature.getName());
         if (operatorType.isPresent()) {
             return new FunctionMetadata(
@@ -796,11 +823,11 @@ private SpecializedFunctionKey getSpecializedFunctionKey(Signature signature)
 
     private SpecializedFunctionKey doGetSpecializedFunctionKey(Signature signature)
     {
-        Iterable<SqlFunction> candidates = getCandidates(null, signature.getName());
+        Iterable<BuiltInFunction> candidates = getFunctions(null, signature.getName());
         // search for exact match
         Type returnType = typeManager.getType(signature.getReturnType());
         List<TypeSignatureProvider> argumentTypeSignatureProviders = fromTypeSignatures(signature.getArgumentTypes());
-        for (SqlFunction candidate : candidates) {
+        for (BuiltInFunction candidate : candidates) {
             Optional<BoundVariables> boundVariables = new SignatureBinder(typeManager, candidate.getSignature(), false)
                     .bindVariables(argumentTypeSignatureProviders, returnType);
             if (boundVariables.isPresent()) {
@@ -811,7 +838,7 @@ private SpecializedFunctionKey doGetSpecializedFunctionKey(Signature signature)
         // TODO: hack because there could be "type only" coercions (which aren't necessarily included as implicit casts),
         // so do a second pass allowing "type only" coercions
         List<Type> argumentTypes = resolveTypes(signature.getArgumentTypes(), typeManager);
-        for (SqlFunction candidate : candidates) {
+        for (BuiltInFunction candidate : candidates) {
             SignatureBinder binder = new SignatureBinder(typeManager, candidate.getSignature(), true);
             Optional<BoundVariables> boundVariables = binder.bindVariables(argumentTypeSignatureProviders, returnType);
             if (!boundVariables.isPresent()) {
@@ -863,25 +890,30 @@ private SpecializedFunctionKey doGetSpecializedFunctionKey(Signature signature)
         throw new PrestoException(FUNCTION_IMPLEMENTATION_MISSING, format("%s not found", signature));
     }
 
+    private static class EmptyTransactionHandle
+            implements FunctionNamespaceTransactionHandle
+    {
+    }
+
     private static class FunctionMap
     {
-        private final Multimap<FullyQualifiedName, SqlFunction> functions;
+        private final Multimap<FullyQualifiedName, BuiltInFunction> functions;
 
         public FunctionMap()
         {
             functions = ImmutableListMultimap.of();
         }
 
-        public FunctionMap(FunctionMap map, Iterable<? extends SqlFunction> functions)
+        public FunctionMap(FunctionMap map, Iterable<? extends BuiltInFunction> functions)
         {
-            this.functions = ImmutableListMultimap.<FullyQualifiedName, SqlFunction>builder()
+            this.functions = ImmutableListMultimap.<FullyQualifiedName, BuiltInFunction>builder()
                     .putAll(map.functions)
                     .putAll(Multimaps.index(functions, function -> function.getSignature().getName()))
                     .build();
 
             // Make sure all functions with the same name are aggregations or none of them are
-            for (Map.Entry<FullyQualifiedName, Collection<SqlFunction>> entry : this.functions.asMap().entrySet()) {
-                Collection<SqlFunction> values = entry.getValue();
+            for (Map.Entry<FullyQualifiedName, Collection<BuiltInFunction>> entry : this.functions.asMap().entrySet()) {
+                Collection<BuiltInFunction> values = entry.getValue();
                 long aggregations = values.stream()
                         .map(function -> function.getSignature().getKind())
                         .filter(kind -> kind == AGGREGATE)
@@ -890,12 +922,12 @@ public FunctionMap(FunctionMap map, Iterable<? extends SqlFunction> functions)
             }
         }
 
-        public List<SqlFunction> list()
+        public List<BuiltInFunction> list()
         {
             return ImmutableList.copyOf(functions.values());
         }
 
-        public Collection<SqlFunction> get(FullyQualifiedName name)
+        public Collection<BuiltInFunction> get(FullyQualifiedName name)
         {
             return functions.get(name);
         }

presto-main/src/main/java/com/facebook/presto/metadata/FunctionExtractor.java

@@ -18,7 +18,6 @@
 import com.facebook.presto.spi.function.AggregationFunction;
 import com.facebook.presto.spi.function.ScalarFunction;
 import com.facebook.presto.spi.function.ScalarOperator;
-import com.facebook.presto.spi.function.SqlFunction;
 import com.facebook.presto.spi.function.WindowFunction;
 
 import java.util.Collection;
@@ -30,15 +29,15 @@ public final class FunctionExtractor
 {
     private FunctionExtractor() {}
 
-    public static List<? extends SqlFunction> extractFunctions(Collection<Class<?>> classes)
+    public static List<BuiltInFunction> extractFunctions(Collection<Class<?>> classes)
     {
         return classes.stream()
                 .map(FunctionExtractor::extractFunctions)
                 .flatMap(Collection::stream)
                 .collect(toImmutableList());
     }
 
-    public static List<? extends SqlFunction> extractFunctions(Class<?> clazz)
+    public static List<? extends BuiltInFunction> extractFunctions(Class<?> clazz)
     {
         if (WindowFunction.class.isAssignableFrom(clazz)) {
             @SuppressWarnings("unchecked")

presto-main/src/main/java/com/facebook/presto/metadata/FunctionListBuilder.java

@@ -15,7 +15,6 @@
 
 import com.facebook.presto.operator.scalar.annotations.ScalarFromAnnotationsParser;
 import com.facebook.presto.operator.window.WindowAnnotationsParser;
-import com.facebook.presto.spi.function.SqlFunction;
 import com.facebook.presto.spi.function.WindowFunction;
 import com.google.common.collect.ImmutableList;
 
@@ -26,7 +25,7 @@
 
 public class FunctionListBuilder
 {
-    private final List<SqlFunction> functions = new ArrayList<>();
+    private final List<BuiltInFunction> functions = new ArrayList<>();
 
     public FunctionListBuilder window(Class<? extends WindowFunction> clazz)
     {
@@ -58,22 +57,22 @@ public FunctionListBuilder scalars(Class<?> clazz)
         return this;
     }
 
-    public FunctionListBuilder functions(SqlFunction... sqlFunctions)
+    public FunctionListBuilder functions(BuiltInFunction... sqlFunctions)
     {
-        for (SqlFunction sqlFunction : sqlFunctions) {
+        for (BuiltInFunction sqlFunction : sqlFunctions) {
             function(sqlFunction);
         }
         return this;
     }
 
-    public FunctionListBuilder function(SqlFunction sqlFunction)
+    public FunctionListBuilder function(BuiltInFunction sqlFunction)
     {
         requireNonNull(sqlFunction, "parametricFunction is null");
         functions.add(sqlFunction);
         return this;
     }
 
-    public List<SqlFunction> getFunctions()
+    public List<BuiltInFunction> getFunctions()
     {
         return ImmutableList.copyOf(functions);
     }