dylib library + changes made to the readme to address comments

presto-native-execution/presto_cpp/main/CMakeLists.txt

@@ -11,6 +11,7 @@
 # limitations under the License.
 add_subdirectory(operators)
 add_subdirectory(types)
+add_subdirectory(functions)
 add_subdirectory(http)
 add_subdirectory(common)
 add_subdirectory(thrift)
@@ -74,7 +75,8 @@ target_link_libraries(
   ${FOLLY_WITH_DEPENDENCIES}
   ${GLOG}
   ${GFLAGS_LIBRARIES}
-  pthread)
+  pthread
+  presto_dynamic_function_loader)
 
 # Enabling Parquet causes build errors with missing symbols on MacOS. This is
 # likely due to a conflict between Arrow Thrift from velox_hive_connector and
@@ -96,8 +98,7 @@ target_link_libraries(presto_server presto_server_lib velox_hive_connector
                       velox_tpch_connector)
 
 if(PRESTO_ENABLE_REMOTE_FUNCTIONS)
-  add_library(presto_server_remote_function JsonSignatureParser.cpp
-                                            RemoteFunctionRegisterer.cpp)
+  add_library(presto_server_remote_function JsonSignatureParser.cpp)
 
   target_link_libraries(presto_server_remote_function velox_expression
                         velox_functions_remote ${FOLLY_WITH_DEPENDENCIES})

presto-native-execution/presto_cpp/main/PrestoServer.cpp

@@ -16,6 +16,7 @@
 #include <boost/asio/ip/host_name.hpp>
 #include <boost/asio/ip/tcp.hpp>
 #include <glog/logging.h>
+#include <iostream>
 #include "CoordinatorDiscoverer.h"
 #include "presto_cpp/main/Announcer.h"
 #include "presto_cpp/main/PeriodicTaskManager.h"
@@ -25,6 +26,7 @@
 #include "presto_cpp/main/common/ConfigReader.h"
 #include "presto_cpp/main/common/Counters.h"
 #include "presto_cpp/main/common/Utils.h"
+#include "presto_cpp/main/functions/DynamicLibraryLoader.h"
 #include "presto_cpp/main/http/filters/AccessLogFilter.h"
 #include "presto_cpp/main/http/filters/HttpEndpointLatencyFilter.h"
 #include "presto_cpp/main/http/filters/InternalAuthenticationFilter.h"
@@ -62,7 +64,7 @@
 #include "velox/serializers/PrestoSerializer.h"
 
 #ifdef PRESTO_ENABLE_REMOTE_FUNCTIONS
-#include "presto_cpp/main/RemoteFunctionRegisterer.h"
+#include "presto_cpp/main/functions/RemoteFunctionRegisterer.h"
 #endif
 
 #ifdef __linux__
@@ -403,6 +405,8 @@ void PrestoServer::run() {
   registerRemoteFunctions();
   registerVectorSerdes();
   registerPrestoPlanNodeSerDe();
+  PRESTO_STARTUP_LOG(INFO) << "heeere 1111!!!";
+  registerDynamicFunctions();
 
   const auto numExchangeHttpClientIoThreads = std::max<size_t>(
       systemConfig->exchangeHttpClientNumIoThreadsHwMultiplier() *
@@ -1444,5 +1448,28 @@ protocol::NodeStatus PrestoServer::fetchNodeStatus() {
 
   return nodeStatus;
 }
+void PrestoServer::registerDynamicFunctions() {
+  auto systemConfig = SystemConfig::instance();
+  PRESTO_STARTUP_LOG(INFO) << "heeere!!!";
+  if (!systemConfig->pluginDir().empty()) {
+    // if it's a valid directory, traverse and call dynamic function loader on
+    // it
+    const fs::path path(systemConfig->pluginDir());
+    PRESTO_STARTUP_LOG(INFO) << path;
+    std::error_code
+        ec; // For using the non-throwing overloads of functions below.
+    if (fs::is_directory(path, ec)) {
+      using recursive_directory_iterator =
+          std::filesystem::recursive_directory_iterator;
+      for (const auto& dirEntry : recursive_directory_iterator(path)) {
+        if (!fs::is_directory(dirEntry, ec)) {
+          loadDynamicLibraryFunctions(dirEntry.path().c_str());
+          std::cout << "LOADED DYLLIB 2" << std::endl;
+          PRESTO_STARTUP_LOG(INFO) << "LOADED DYLLIB 2";
+        }
+      }
+    }
+  }
+}
 
 } // namespace facebook::presto

presto-native-execution/presto_cpp/main/PrestoServer.h

@@ -186,6 +186,8 @@ class PrestoServer {
 
   VeloxPlanValidator* getVeloxPlanValidator();
 
+  virtual void registerDynamicFunctions();
+
   /// Invoked to get the list of filters passed to the http server.
   std::vector<std::unique_ptr<proxygen::RequestHandlerFactory>>
   getHttpServerFilters();

presto-native-execution/presto_cpp/main/common/Configs.cpp

@@ -233,6 +233,7 @@ SystemConfig::SystemConfig() {
           STR_PROP(kCacheVeloxTtlCheckInterval, "1h"),
           BOOL_PROP(kEnableRuntimeMetricsCollection, false),
           BOOL_PROP(kPlanValidatorFailOnNestedLoopJoin, false),
+          STR_PROP(kPluginDir, ""),
       };
 }
 
@@ -734,6 +735,10 @@ bool SystemConfig::enableRuntimeMetricsCollection() const {
   return optionalProperty<bool>(kEnableRuntimeMetricsCollection).value();
 }
 
+std::string SystemConfig::pluginDir() const {
+  return optionalProperty(kPluginDir).value();
+}
+
 NodeConfig::NodeConfig() {
   registeredProps_ =
       std::unordered_map<std::string, folly::Optional<std::string>>{

presto-native-execution/presto_cpp/main/common/Configs.h

@@ -617,6 +617,8 @@ class SystemConfig : public ConfigBase {
   static constexpr std::string_view kInternalCommunicationJwtExpirationSeconds{
       "internal-communication.jwt.expiration-seconds"};
 
+  /// Optional string containing the path to the plugin directory
+  static constexpr std::string_view kPluginDir{"plugin.dir"};
   /// Below are the Presto properties from config.properties that get converted
   /// to their velox counterparts in BaseVeloxQueryConfig and used solely from
   /// BaseVeloxQueryConfig.
@@ -856,6 +858,8 @@ class SystemConfig : public ConfigBase {
   bool enableRuntimeMetricsCollection() const;
 
   bool prestoNativeSidecar() const;
+
+  std::string pluginDir() const;
 };
 
 /// Provides access to node properties defined in node.properties file.

presto-native-execution/presto_cpp/main/functions/CMakeLists.txt

@@ -0,0 +1,13 @@
+add_library(presto_dynamic_function_loader OBJECT DynamicLibraryLoader.cpp)
+
+if(PRESTO_ENABLE_TESTING)
+  add_subdirectory(tests)
+endif()
+
+if(PRESTO_ENABLE_REMOTE_FUNCTIONS)
+  add_library(presto_server_remote_function RemoteFunctionRegisterer.cpp)
+
+  target_link_libraries(presto_server_remote_function velox_expression
+                        velox_functions_remote ${FOLLY_WITH_DEPENDENCIES})
+  target_link_libraries(presto_server_lib presto_server_remote_function)
+endif()

presto-native-execution/presto_cpp/main/functions/DynamicLibraryLoader.cpp

@@ -0,0 +1,45 @@
+/*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * 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.
+ */
+
+#include "presto_cpp/main/functions/DynamicLibraryLoader.h"
+#include <dlfcn.h>
+#include <iostream>
+#include "velox/common/base/Exceptions.h"
+namespace facebook::presto {
+
+static constexpr const char* kSymbolName = "registry";
+
+void loadDynamicLibraryFunctions(const char* fileName) {
+  // Try to dynamically load the shared library.
+  void* handler = dlopen(fileName, RTLD_NOW);
+
+  if (handler == nullptr) {
+    VELOX_USER_FAIL("Error while loading shared library: {}", dlerror());
+  }
+
+  // Lookup the symbol.
+  void* registrySymbol = dlsym(handler, kSymbolName);
+  auto registryFunction = reinterpret_cast<void (*)()>(registrySymbol);
+  char* error = dlerror();
+
+  if (error != nullptr) {
+    VELOX_USER_FAIL("Couldn't find Velox registry symbol: {}", error);
+  }
+  registryFunction();
+  std::cout << "LOADED DYLLIB 1" << std::endl;
+}
+
+} // namespace facebook::presto

presto-native-execution/presto_cpp/main/functions/DynamicLibraryLoader.h

@@ -0,0 +1,37 @@
+/*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * 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.
+ */
+
+#pragma once
+
+namespace facebook::presto {
+
+/// Dynamically opens and registers functions defined in a shared library (.so)
+///
+/// Given a shared library name (.so), this function will open it using dlopen,
+/// search for a "void registry()" C function containing the registration code
+/// for the functions defined in library, and execute it.
+///
+/// The library being linked needs to provide a function with the following
+/// signature:
+///
+///   void registry();
+///
+/// The registration function needs to be defined in the top-level namespace,
+/// and be enclosed by a extern "C" directive to prevent the compiler from
+/// mangling the symbol name.
+void loadDynamicLibraryFunctions(const char* fileName);
+
+} // namespace facebook::presto

presto-native-execution/presto_cpp/main/functions/README.md

@@ -0,0 +1,23 @@
+# Prestissimo: Dynamically Loading Function Library
+
+This library adds the ability to load User Defined Functions (UDFs) without having to fork and build Prestissimo, through the use of shared libraries that a Prestissimo worker can access. The dynamic functions are to be loaded on launch of the Presto server. The Presto server searches for any .so or .dylib files and loads them using this library.
+
+## Getting started
+
+The Process can be roughly broken down into the steps below:
+### 1. Create a cpp file for your dynamic library
+For dynamically loaded function registration, the format followed is mirrored of that of built-in function registration with some noted differences. Using [MyDynamicTestFunction.cpp](tests/MyDynamicTestFunction.cpp) as an example, the function uses the extern "C" keyword to protect against name mangling. A registry() function call is also necessary here.
+
+### 2. Register functions dynamically by creating .dylib or .so shared libraries and dropping them in a plugin directory. 
+These shared libraries may be made using CMakeLists like the following:
+```
+add_library(name_of_dynamic_fn SHARED TestFunction.cpp)
+target_link_libraries(name_of_dynamic_fn PRIVATE xsimd fmt::fmt velox_expression)
+```
+
+### 3. In the Prestissimo worker's config.properties file, set the plugin.dir property
+This is where you will be dropping your shared library.
+```
+plugin.dir="User\Test\Path\plugin"
+```
+### 4. When the worker or the sidecar process starts, it will scan the plugin directory and attempt to dynamically load all shared libraries.

presto-native-execution/presto_cpp/main/RemoteFunctionRegisterer.cpp → presto-native-execution/presto_cpp/main/functions/RemoteFunctionRegisterer.cpp

@@ -12,7 +12,7 @@
  * limitations under the License.
  */
 
-#include "presto_cpp/main/RemoteFunctionRegisterer.h"
+#include "presto_cpp/main/functions/RemoteFunctionRegisterer.h"
 #include <fstream>
 #include "presto_cpp/main/JsonSignatureParser.h"
 #include "velox/common/base/Exceptions.h"

presto-native-execution/presto_cpp/main/functions/tests/CMakeLists.txt

@@ -0,0 +1,27 @@
+# To test functions being added by dynamically linked libraries, we compile
+# `MyDynamicTestFunction.cpp` as a small .so library, and use the
+# MY_DYNAMIC_FUNCTION_LIBRARY_PATH macro to locate the .so binary.
+add_compile_definitions(
+  MY_DYNAMIC_FUNCTION_LIBRARY_PATH="${CMAKE_CURRENT_BINARY_DIR}")
+add_library(presto_function_my_dynamic SHARED MyDynamicTestFunction.cpp)
+target_link_libraries(presto_function_my_dynamic PRIVATE xsimd fmt::fmt
+                                                         velox_expression)
+
+# Here's the actual test which will dynamically load the library defined above.
+add_executable(presto_function_dynamic_link_test DynamicLinkTest.cpp)
+
+add_test(NAME presto_function_dynamic_link_test
+         COMMAND presto_function_dynamic_link_test)
+
+target_link_libraries(
+  presto_function_dynamic_link_test
+  velox_functions_test_lib
+  presto_dynamic_function_loader
+  velox_function_registry
+  xsimd
+  gmock
+  ${GTEST_BOTH_LIBRARIES})
+
+# these are the testing flags all in a dynamic library
+add_library(presto_flags SHARED FlagLibrary.cpp)
+target_link_libraries(presto_flags gflags::gflags)

presto-native-execution/presto_cpp/main/functions/tests/DynamicLinkTest.cpp

@@ -0,0 +1,57 @@
+/*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * 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.
+ */
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+#include "presto_cpp/main/functions/DynamicLibraryLoader.h"
+#include "velox/common/base/Exceptions.h"
+#include "velox/functions/FunctionRegistry.h"
+#include "velox/functions/prestosql/tests/utils/FunctionBaseTest.h"
+
+using namespace facebook::velox::functions::test;
+using namespace facebook::velox;
+using namespace facebook::presto;
+namespace facebook::presto::functions::test {
+
+class DynamicLinkTest : public FunctionBaseTest {};
+
+TEST_F(DynamicLinkTest, dynamicLoad) {
+  const auto dynamicFunction = [&](std::optional<double> a) {
+    return evaluateOnce<int64_t>("dynamic_123()", a);
+  };
+
+  auto signaturesBefore = getFunctionSignatures().size();
+
+  // Function does not exist yet.
+  EXPECT_THROW(dynamicFunction(0), VeloxUserError);
+
+  // Dynamically load the library.
+  std::string libraryPath = MY_DYNAMIC_FUNCTION_LIBRARY_PATH;
+  libraryPath +=
+      "/libpresto_function_my_dynamic.dylib"; // building on MacOS leads to
+                                              // .dylib file not .so file
+
+  loadDynamicLibraryFunctions(libraryPath.data());
+
+  auto signaturesAfter = getFunctionSignatures().size();
+  EXPECT_EQ(signaturesAfter, signaturesBefore + 1);
+
+  // Make sure the function exists now.
+  EXPECT_EQ(123, dynamicFunction(0));
+}
+
+} // namespace facebook::presto::functions::test