Fix review comments

CMakeLists.txt

@@ -138,7 +138,7 @@ option(VELOX_ENABLE_ABFS "Build Abfs Connector" OFF)
 option(VELOX_ENABLE_HDFS "Build Hdfs Connector" OFF)
 option(VELOX_ENABLE_PARQUET "Enable Parquet support" ON)
 option(VELOX_ENABLE_ARROW "Enable Arrow support" OFF)
-option(VELOX_ENABLE_REMOTE_FUNCTIONS "Enable remote function support" ON)
+option(VELOX_ENABLE_REMOTE_FUNCTIONS "Enable remote function support" OFF)
 option(VELOX_ENABLE_CCACHE "Use ccache if installed." ON)
 
 option(VELOX_BUILD_TEST_UTILS "Builds Velox test utilities" OFF)

velox/functions/remote/client/tests/RemoteFunctionRestTest.cpp

@@ -47,7 +47,7 @@ class RemoteFunctionRestTest
   // Registers a few remote functions to be used in this test.
   void registerRemoteFunctions() const {
     RemoteVectorFunctionMetadata metadata;
-    metadata.serdeFormat = GetParam();
+    metadata.serdeFormat = remote::PageFormat::PRESTO_PAGE;
     metadata.location = location_;
 
     auto absSignature = {exec::FunctionSignatureBuilder()
@@ -64,7 +64,7 @@ class RemoteFunctionRestTest
     registerRemoteFunction("remote_plus", plusSignatures, metadata);
 
     RemoteVectorFunctionMetadata wrongMetadata = metadata;
-    wrongMetadata.serdeFormat = GetParam();
+    wrongMetadata.serdeFormat = remote::PageFormat::PRESTO_PAGE;
     wrongMetadata.location = wrongLocation_;
     registerRemoteFunction("remote_wrong_port", plusSignatures, wrongMetadata);
 
@@ -152,7 +152,7 @@ class RemoteFunctionRestTest
   const std::string remotePrefix_{"remote"};
 };
 
-TEST_P(RemoteFunctionRestTest, absolute) {
+TEST_F(RemoteFunctionRestTest, absolute) {
   auto inputVector = makeFlatVector<int32_t>({-10, -20});
   auto results = evaluate<SimpleVector<int32_t>>(
       "remote_abs(c0)", makeRowVector({inputVector}));
@@ -161,7 +161,7 @@ TEST_P(RemoteFunctionRestTest, absolute) {
   assertEqualVectors(expected, results);
 }
 
-TEST_P(RemoteFunctionRestTest, simple) {
+TEST_F(RemoteFunctionRestTest, simple) {
   auto inputVector = makeFlatVector<int64_t>({1, 2, 3, 4, 5});
   auto results = evaluate<SimpleVector<int64_t>>(
       "remote_plus(c0, c0)", makeRowVector({inputVector}));
@@ -170,7 +170,7 @@ TEST_P(RemoteFunctionRestTest, simple) {
   assertEqualVectors(expected, results);
 }
 
-TEST_P(RemoteFunctionRestTest, string) {
+TEST_F(RemoteFunctionRestTest, string) {
   auto inputVector =
       makeFlatVector<StringView>({"hello", "my", "remote", "world"});
   auto inputVector1 = makeFlatVector<int32_t>({2, 1, 3, 5});
@@ -181,7 +181,7 @@ TEST_P(RemoteFunctionRestTest, string) {
   assertEqualVectors(expected, results);
 }
 
-TEST_P(RemoteFunctionRestTest, connectionError) {
+TEST_F(RemoteFunctionRestTest, connectionError) {
   auto inputVector = makeFlatVector<int64_t>({1, 2, 3, 4, 5});
   auto func = [&]() {
     evaluate<SimpleVector<int64_t>>(
@@ -200,11 +200,6 @@ TEST_P(RemoteFunctionRestTest, connectionError) {
   }
 }
 
-VELOX_INSTANTIATE_TEST_SUITE_P(
-    RemoteFunctionRestTestFixture,
-    RemoteFunctionRestTest,
-    ::testing::Values(remote::PageFormat::PRESTO_PAGE));
-
 } // namespace
 } // namespace facebook::velox::functions
 

velox/functions/remote/server/RemoteFunctionServiceMain.cpp

@@ -18,7 +18,6 @@
 #include <gflags/gflags.h>
 #include <glog/logging.h>
 #include <thrift/lib/cpp2/server/ThriftServer.h>
-#include "velox/functions/prestosql/StringFunctions.h"
 #include "velox/functions/prestosql/registration/RegistrationFunctions.h"
 #include "velox/functions/remote/server/RemoteFunctionService.h"
 
@@ -37,7 +36,7 @@ DEFINE_string(
 
 DEFINE_string(
     function_prefix,
-    "remote.schema.",
+    "json.test_schema.",
     "Prefix to be added to the functions being registered");
 
 using namespace ::facebook::velox;
@@ -47,14 +46,11 @@ int main(int argc, char* argv[]) {
   folly::Init init{&argc, &argv, false};
   FLAGS_logtostderr = true;
 
-  memory::initializeMemoryManager({});
-
   // Always registers all Presto functions and make them available under a
   // certain prefix/namespace.
   LOG(INFO) << "Registering Presto functions";
   functions::prestosql::registerAllScalarFunctions(FLAGS_function_prefix);
 
-  std::remove(FLAGS_uds_path.c_str());
   folly::SocketAddress location{
       folly::SocketAddress::makeFromPath(FLAGS_uds_path)};