From 1d753537fbc5391412c9e27f48f94a17c6cca081 Mon Sep 17 00:00:00 2001 From: mohsaka <135669458+mohsaka@users.noreply.github.com> Date: Fri, 31 Jan 2025 15:13:51 -0800 Subject: [PATCH] Address comments --- velox/common/dynamic_registry/README.md | 8 ++++-- .../tests/MyDynamicErrFunction.cpp | 3 +-- .../tests/MyDynamicFunction.cpp | 3 +-- .../tests/MyDynamicIntFunctionOverload.cpp | 3 +-- .../tests/MyDynamicIntFunctionOverwrite.cpp | 3 +-- .../MyDynamicVarcharFunctionOverload.cpp | 3 +-- .../MyDynamicVarcharFunctionOverwrite.cpp | 3 +-- velox/functions/DynamicUdf.h | 26 +++++++++++++++++++ 8 files changed, 38 insertions(+), 14 deletions(-) create mode 100644 velox/functions/DynamicUdf.h diff --git a/velox/common/dynamic_registry/README.md b/velox/common/dynamic_registry/README.md index 391a8a64c2a3..d3e128910a6d 100644 --- a/velox/common/dynamic_registry/README.md +++ b/velox/common/dynamic_registry/README.md @@ -4,7 +4,11 @@ This generic utility adds extensibility features to load User Defined Functions ## Getting started 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. +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. +Make sure to also include the necessary header file: + ``` + #include "velox/functions/DynamicUdf.h" + ``` 2. Register functions dynamically by creating .dylib (MacOS) or .so (Linux) shared libraries. These shared libraries may be made using CMakeLists like the following: @@ -19,7 +23,7 @@ These shared libraries may be made using CMakeLists like the following: * MacOS: ``` add_library(name_of_dynamic_fn SHARED TestFunction.cpp) - target_link_libraries(name_of_dynamic_fn PRIVATE fmt::fmt Folly::folly gflags::gflags xsimd) + target_link_libraries(name_of_dynamic_fn PRIVATE Folly::folly xsimd) ``` Additionally, the below flag allows symbols to be resolved at runtime: diff --git a/velox/common/dynamic_registry/tests/MyDynamicErrFunction.cpp b/velox/common/dynamic_registry/tests/MyDynamicErrFunction.cpp index 4de6032032de..8b3338b25e36 100644 --- a/velox/common/dynamic_registry/tests/MyDynamicErrFunction.cpp +++ b/velox/common/dynamic_registry/tests/MyDynamicErrFunction.cpp @@ -13,8 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -#include "velox/functions/Macros.h" -#include "velox/functions/Registerer.h" +#include "velox/functions/DynamicUdf.h" // This file defines a mock function that will be dynamically linked and // registered. There are no restrictions as to how the function needs to be diff --git a/velox/common/dynamic_registry/tests/MyDynamicFunction.cpp b/velox/common/dynamic_registry/tests/MyDynamicFunction.cpp index 51156276134c..6401c9a4f267 100644 --- a/velox/common/dynamic_registry/tests/MyDynamicFunction.cpp +++ b/velox/common/dynamic_registry/tests/MyDynamicFunction.cpp @@ -13,8 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -#include "velox/functions/Macros.h" -#include "velox/functions/Registerer.h" +#include "velox/functions/DynamicUdf.h" // This file defines a mock function that will be dynamically linked and // registered. There are no restrictions as to how the function needs to be diff --git a/velox/common/dynamic_registry/tests/MyDynamicIntFunctionOverload.cpp b/velox/common/dynamic_registry/tests/MyDynamicIntFunctionOverload.cpp index 782177bd61eb..6b6ada39c6d4 100644 --- a/velox/common/dynamic_registry/tests/MyDynamicIntFunctionOverload.cpp +++ b/velox/common/dynamic_registry/tests/MyDynamicIntFunctionOverload.cpp @@ -13,8 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -#include "velox/functions/Macros.h" -#include "velox/functions/Registerer.h" +#include "velox/functions/DynamicUdf.h" // This file defines a mock function that will be dynamically linked and // registered. There are no restrictions as to how the function needs to be diff --git a/velox/common/dynamic_registry/tests/MyDynamicIntFunctionOverwrite.cpp b/velox/common/dynamic_registry/tests/MyDynamicIntFunctionOverwrite.cpp index fc675bf23a62..a31cd96631e0 100644 --- a/velox/common/dynamic_registry/tests/MyDynamicIntFunctionOverwrite.cpp +++ b/velox/common/dynamic_registry/tests/MyDynamicIntFunctionOverwrite.cpp @@ -13,8 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -#include "velox/functions/Macros.h" -#include "velox/functions/Registerer.h" +#include "velox/functions/DynamicUdf.h" // This file defines a mock function that will be dynamically linked and // registered. There are no restrictions as to how the function needs to be diff --git a/velox/common/dynamic_registry/tests/MyDynamicVarcharFunctionOverload.cpp b/velox/common/dynamic_registry/tests/MyDynamicVarcharFunctionOverload.cpp index 3a29e5d386de..a25c3bc7dad5 100644 --- a/velox/common/dynamic_registry/tests/MyDynamicVarcharFunctionOverload.cpp +++ b/velox/common/dynamic_registry/tests/MyDynamicVarcharFunctionOverload.cpp @@ -13,8 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -#include "velox/functions/Macros.h" -#include "velox/functions/Registerer.h" +#include "velox/functions/DynamicUdf.h" // This file defines a mock function that will be dynamically linked and // registered. There are no restrictions as to how the function needs to be diff --git a/velox/common/dynamic_registry/tests/MyDynamicVarcharFunctionOverwrite.cpp b/velox/common/dynamic_registry/tests/MyDynamicVarcharFunctionOverwrite.cpp index da4e9f9d19a3..8c00ae73df5c 100644 --- a/velox/common/dynamic_registry/tests/MyDynamicVarcharFunctionOverwrite.cpp +++ b/velox/common/dynamic_registry/tests/MyDynamicVarcharFunctionOverwrite.cpp @@ -13,8 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -#include "velox/functions/Macros.h" -#include "velox/functions/Registerer.h" +#include "velox/functions/DynamicUdf.h" // This file defines a mock function that will be dynamically linked and // registered. There are no restrictions as to how the function needs to be diff --git a/velox/functions/DynamicUdf.h b/velox/functions/DynamicUdf.h new file mode 100644 index 000000000000..8db3b5887d5f --- /dev/null +++ b/velox/functions/DynamicUdf.h @@ -0,0 +1,26 @@ +/* + * 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 + +#include "velox/functions/Udf.h" + +extern "C"{ +void registry(); +void check(){ + registry(); +} +} +