From 12b5c58d031ec3468fb26462561bd32d96f1b9d8 Mon Sep 17 00:00:00 2001 From: Deepak Majeti Date: Mon, 19 Feb 2024 16:30:16 +0530 Subject: [PATCH] Add support to provide an alias for a connector factory --- velox/connectors/Connector.cpp | 13 +++++ velox/connectors/Connector.h | 7 +++ velox/connectors/hive/HiveConnector.cpp | 3 +- velox/connectors/hive/HiveConnector.h | 11 ----- velox/connectors/hive/tests/CMakeLists.txt | 1 + .../hive/tests/HiveConnectorFactoryTest.cpp | 48 +++++++++++++++++++ 6 files changed, 70 insertions(+), 13 deletions(-) create mode 100644 velox/connectors/hive/tests/HiveConnectorFactoryTest.cpp diff --git a/velox/connectors/Connector.cpp b/velox/connectors/Connector.cpp index 7d972c6670d..b75599fb99f 100644 --- a/velox/connectors/Connector.cpp +++ b/velox/connectors/Connector.cpp @@ -54,6 +54,19 @@ bool registerConnectorFactory(std::shared_ptr factory) { return true; } +bool registerConnectorFactoryAlias( + const std::string& alias, + const std::string& connectorId) { + auto factory = getConnectorFactory(connectorId); + bool ok = connectorFactories().insert({alias, factory}).second; + VELOX_CHECK( + ok, + "Alias '{}' is already registered for factory '{}'", + alias, + connectorId); + return true; +} + std::shared_ptr getConnectorFactory( const std::string& connectorName) { auto it = connectorFactories().find(connectorName); diff --git a/velox/connectors/Connector.h b/velox/connectors/Connector.h index 06b705d7b02..b03a9e91b02 100644 --- a/velox/connectors/Connector.h +++ b/velox/connectors/Connector.h @@ -421,6 +421,13 @@ class ConnectorFactory { /// FB_ANONYMOUS_VARIABLE. bool registerConnectorFactory(std::shared_ptr factory); +/// Adds an alias for an existing factory. Throws if factory with the +/// connectorName is not already present. Throws if the alias for the +/// connectorName is already present. +bool registerConnectorFactoryAlias( + const std::string& alias, + const std::string& connectorName); + /// Returns a factory for creating connectors with the specified name. Throws if /// factory doesn't exist. std::shared_ptr getConnectorFactory( diff --git a/velox/connectors/hive/HiveConnector.cpp b/velox/connectors/hive/HiveConnector.cpp index 9b2af5d0793..a11efc901df 100644 --- a/velox/connectors/hive/HiveConnector.cpp +++ b/velox/connectors/hive/HiveConnector.cpp @@ -220,6 +220,5 @@ void registerHivePartitionFunctionSerDe() { } VELOX_REGISTER_CONNECTOR_FACTORY(std::make_shared()) -VELOX_REGISTER_CONNECTOR_FACTORY( - std::make_shared()) + } // namespace facebook::velox::connector::hive diff --git a/velox/connectors/hive/HiveConnector.h b/velox/connectors/hive/HiveConnector.h index 6232e418465..3cf1250fe98 100644 --- a/velox/connectors/hive/HiveConnector.h +++ b/velox/connectors/hive/HiveConnector.h @@ -83,14 +83,9 @@ class HiveConnector : public Connector { class HiveConnectorFactory : public ConnectorFactory { public: static constexpr const char* FOLLY_NONNULL kHiveConnectorName = "hive"; - static constexpr const char* FOLLY_NONNULL kHiveHadoop2ConnectorName = - "hive-hadoop2"; HiveConnectorFactory() : ConnectorFactory(kHiveConnectorName) {} - explicit HiveConnectorFactory(const char* FOLLY_NONNULL connectorName) - : ConnectorFactory(connectorName) {} - /// Register HiveConnector components such as Dwrf, Parquet readers and /// writers and FileSystems. void initialize() override; @@ -103,12 +98,6 @@ class HiveConnectorFactory : public ConnectorFactory { } }; -class HiveHadoop2ConnectorFactory : public HiveConnectorFactory { - public: - HiveHadoop2ConnectorFactory() - : HiveConnectorFactory(kHiveHadoop2ConnectorName) {} -}; - class HivePartitionFunctionSpec : public core::PartitionFunctionSpec { public: HivePartitionFunctionSpec( diff --git a/velox/connectors/hive/tests/CMakeLists.txt b/velox/connectors/hive/tests/CMakeLists.txt index f84b2eb8ccf..f7108a89d29 100644 --- a/velox/connectors/hive/tests/CMakeLists.txt +++ b/velox/connectors/hive/tests/CMakeLists.txt @@ -17,6 +17,7 @@ add_executable( HivePartitionFunctionTest.cpp FileHandleTest.cpp HivePartitionUtilTest.cpp + HiveConnectorFactoryTest.cpp HiveConnectorTest.cpp HiveConnectorUtilTest.cpp HiveConnectorSerDeTest.cpp diff --git a/velox/connectors/hive/tests/HiveConnectorFactoryTest.cpp b/velox/connectors/hive/tests/HiveConnectorFactoryTest.cpp new file mode 100644 index 00000000000..61c9dcfddfc --- /dev/null +++ b/velox/connectors/hive/tests/HiveConnectorFactoryTest.cpp @@ -0,0 +1,48 @@ +/* + * 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 + +#include "velox/common/base/tests/GTestUtils.h" +#include "velox/connectors/Connector.h" + +namespace facebook::velox::connector::hive { +namespace { + +TEST(HiveConnectorFactoryTest, aliases) { + auto factory = connector::getConnectorFactory("hive"); + ASSERT_EQ(factory->connectorName(), "hive"); + + registerConnectorFactoryAlias("hive-hadoop2", factory->connectorName()); + registerConnectorFactoryAlias("iceberg", factory->connectorName()); + + ASSERT_EQ( + connector::getConnectorFactory("hive-hadoop2")->connectorName(), + factory->connectorName()); + ASSERT_EQ( + connector::getConnectorFactory("iceberg")->connectorName(), + factory->connectorName()); + + VELOX_ASSERT_THROW( + registerConnectorFactoryAlias("iceberg", factory->connectorName()), + "Alias 'iceberg' is already registered for factory 'hive'"); + VELOX_ASSERT_THROW( + registerConnectorFactoryAlias("foo", "hivefoo"), + "ConnectorFactory with name 'hivefoo' not registered"); +} + +} // namespace +} // namespace facebook::velox::connector::hive