rename memory catalog

Author: gty404

src/iceberg/CMakeLists.txt

@@ -19,7 +19,7 @@ set(ICEBERG_INCLUDES "$<BUILD_INTERFACE:${PROJECT_BINARY_DIR}/src>"
                      "$<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/src>")
 set(ICEBERG_SOURCES
     arrow_c_data_internal.cc
-    catalog/memory_catalog.cc
+    catalog/in_memory_catalog.cc
     demo.cc
     expression/expression.cc
     file_reader.cc

src/iceberg/catalog.h

@@ -90,11 +90,11 @@ class ICEBERG_EXPORT Catalog {
   /// \brief Check whether table exists
   ///
   /// \param identifier a table identifier
-  /// \return Status indicating success or failure.
+  /// \return Result<bool> indicating table exists or not.
   ///         - On success, the table existence was successfully checked (actual existence
   ///         may be inferred elsewhere).
   ///         - On failure, contains error information.
-  virtual Status TableExists(const TableIdentifier& identifier) const = 0;
+  virtual Result<bool> TableExists(const TableIdentifier& identifier) const = 0;
 
   /// \brief Drop a table; optionally delete data and metadata files
   ///
@@ -124,18 +124,6 @@ class ICEBERG_EXPORT Catalog {
   virtual Result<std::shared_ptr<Table>> RegisterTable(
       const TableIdentifier& identifier, const std::string& metadata_file_location) = 0;
 
-  /// \brief Initialize a catalog given a custom name and a map of catalog properties
-  ///
-  /// A custom Catalog implementation must have a default constructor. A compute engine
-  /// will first initialize the catalog without any arguments, and then call this method
-  /// to complete catalog initialization with properties passed into the engine.
-  ///
-  /// \param name a custom name for the catalog
-  /// \param properties catalog properties
-  virtual void Initialize(
-      const std::string& name,
-      const std::unordered_map<std::string, std::string>& properties) = 0;
-
   /// \brief Instantiate a builder to either create a table or start a create/replace
   /// transaction
   ///

src/iceberg/catalog/memory_catalog.cc renamed to src/iceberg/catalog/in_memory_catalog.cc

@@ -17,7 +17,7 @@
  * under the License.
  */
 
-#include "iceberg/catalog/memory_catalog.h"
+#include "iceberg/catalog/in_memory_catalog.h"
 
 #include <algorithm>
 #include <iterator>  // IWYU pragma: keep
@@ -42,19 +42,15 @@ Result<const InMemoryNamespace*> GetNamespace(const InMemoryNamespace* root,
 
 }  // namespace
 
-InMemoryCatalog::InMemoryCatalog(std::shared_ptr<FileIO> file_io,
-                                 std::string warehouse_location)
-    : file_io_(std::move(file_io)),
+InMemoryCatalog::InMemoryCatalog(std::string name, std::shared_ptr<FileIO> file_io,
+                                 std::string warehouse_location,
+                                 std::unordered_map<std::string, std::string> properties)
+    : catalog_name_(std::move(name)),
+      properties_(std::move(properties)),
+      file_io_(std::move(file_io)),
       warehouse_location_(std::move(warehouse_location)),
       root_namespace_(std::make_unique<InMemoryNamespace>()) {}
 
-void InMemoryCatalog::Initialize(
-    const std::string& name,
-    const std::unordered_map<std::string, std::string>& properties) {
-  catalog_name_ = name;
-  properties_ = properties;
-}
-
 std::string_view InMemoryCatalog::name() const { return catalog_name_; }
 
 Result<std::vector<TableIdentifier>> InMemoryCatalog::ListTables(
@@ -74,27 +70,24 @@ Result<std::unique_ptr<Table>> InMemoryCatalog::CreateTable(
     const TableIdentifier& identifier, const Schema& schema, const PartitionSpec& spec,
     const std::string& location,
     const std::unordered_map<std::string, std::string>& properties) {
-  return unexpected<Error>(
-      {.kind = ErrorKind::kNotImplemented, .message = "CreateTable"});
+  return NotImplemented("create table");
 }
 
 Result<std::unique_ptr<Table>> InMemoryCatalog::UpdateTable(
     const TableIdentifier& identifier,
     const std::vector<std::unique_ptr<UpdateRequirement>>& requirements,
     const std::vector<std::unique_ptr<MetadataUpdate>>& updates) {
-  return unexpected<Error>(
-      {.kind = ErrorKind::kNotImplemented, .message = "UpdateTable"});
+  return NotImplemented("update table");
 }
 
 Result<std::shared_ptr<Transaction>> InMemoryCatalog::StageCreateTable(
     const TableIdentifier& identifier, const Schema& schema, const PartitionSpec& spec,
     const std::string& location,
     const std::unordered_map<std::string, std::string>& properties) {
-  return unexpected<Error>(
-      {.kind = ErrorKind::kNotImplemented, .message = "StageCreateTable"});
+  return NotImplemented("stage create table");
 }
 
-Status InMemoryCatalog::TableExists(const TableIdentifier& identifier) const {
+Result<bool> InMemoryCatalog::TableExists(const TableIdentifier& identifier) const {
   std::unique_lock lock(mutex_);
   return root_namespace_->TableExists(identifier);
 }
@@ -107,19 +100,17 @@ Status InMemoryCatalog::DropTable(const TableIdentifier& identifier, bool purge)
 
 Result<std::shared_ptr<Table>> InMemoryCatalog::LoadTable(
     const TableIdentifier& identifier) const {
-  return unexpected<Error>({.kind = ErrorKind::kNotImplemented, .message = "LoadTable"});
+  return NotImplemented("load table");
 }
 
 Result<std::shared_ptr<Table>> InMemoryCatalog::RegisterTable(
     const TableIdentifier& identifier, const std::string& metadata_file_location) {
   std::unique_lock lock(mutex_);
   if (!root_namespace_->NamespaceExists(identifier.ns)) {
-    return unexpected<Error>({.kind = ErrorKind::kNoSuchNamespace,
-                              .message = "table namespace does not exist"});
+    return NoSuchNamespace("table namespace does not exist.");
   }
   if (!root_namespace_->RegisterTable(identifier, metadata_file_location)) {
-    return unexpected<Error>(
-        {.kind = ErrorKind::kUnknownError, .message = "The registry failed."});
+    return UnknownError("The registry failed.");
   }
   return LoadTable(identifier);
 }
@@ -253,13 +244,10 @@ Status InMemoryNamespace::UnregisterTable(TableIdentifier const& table_ident) {
   return {};
 }
 
-Status InMemoryNamespace::TableExists(TableIdentifier const& table_ident) const {
+Result<bool> InMemoryNamespace::TableExists(TableIdentifier const& table_ident) const {
   const auto ns = GetNamespace(this, table_ident.ns);
   ICEBERG_RETURN_UNEXPECTED(ns);
-  if (!ns.value()->table_metadata_locations_.contains(table_ident.name)) {
-    return NotFound("{} does not exist", table_ident.name);
-  }
-  return {};
+  return ns.value()->table_metadata_locations_.contains(table_ident.name);
 }
 
 Result<std::string> InMemoryNamespace::GetTableMetadataLocation(

src/iceberg/catalog/memory_catalog.h renamed to src/iceberg/catalog/in_memory_catalog.h

@@ -27,15 +27,11 @@
 
 namespace iceberg {
 
-class InMemoryNamespace;
-
 class ICEBERG_EXPORT InMemoryCatalog : public Catalog {
  public:
-  InMemoryCatalog(std::shared_ptr<FileIO> file_io, std::string warehouse_location);
-
-  void Initialize(
-      const std::string& name,
-      const std::unordered_map<std::string, std::string>& properties) override;
+  InMemoryCatalog(std::string name, std::shared_ptr<FileIO> file_io,
+                  std::string warehouse_location,
+                  std::unordered_map<std::string, std::string> properties);
 
   std::string_view name() const override;
 
@@ -56,7 +52,7 @@ class ICEBERG_EXPORT InMemoryCatalog : public Catalog {
       const std::string& location,
       const std::unordered_map<std::string, std::string>& properties) override;
 
-  Status TableExists(const TableIdentifier& identifier) const override;
+  Result<bool> TableExists(const TableIdentifier& identifier) const override;
 
   Status DropTable(const TableIdentifier& identifier, bool purge) override;
 
@@ -75,7 +71,7 @@ class ICEBERG_EXPORT InMemoryCatalog : public Catalog {
   std::unordered_map<std::string, std::string> properties_;
   std::shared_ptr<FileIO> file_io_;
   std::string warehouse_location_;
-  std::unique_ptr<InMemoryNamespace> root_namespace_;
+  std::unique_ptr<class InMemoryNamespace> root_namespace_;
   mutable std::recursive_mutex mutex_;
 };
 
@@ -165,9 +161,9 @@ class ICEBERG_EXPORT InMemoryNamespace {
   /**
    * \brief Checks if a table exists in the specified namespace.
    * \param[in] table_ident The identifier of the table to check.
-   * \return Status indicating success or failure.
+   * \return Result<bool> indicating table exists or not.
    */
-  Status TableExists(TableIdentifier const& table_ident) const;
+  Result<bool> TableExists(TableIdentifier const& table_ident) const;
 
   /**
    * \brief Gets the metadata location for the specified table.

test/CMakeLists.txt

@@ -43,7 +43,7 @@ target_link_libraries(schema_test PRIVATE iceberg_static GTest::gtest_main GTest
 add_test(NAME schema_test COMMAND schema_test)
 
 add_executable(catalog_test)
-target_sources(catalog_test PRIVATE memory_catalog_test.cc)
+target_sources(catalog_test PRIVATE in_memory_catalog_test.cc)
 target_link_libraries(catalog_test PRIVATE iceberg_static GTest::gtest_main GTest::gmock)
 add_test(NAME catalog_test COMMAND catalog_test)
 

test/memory_catalog_test.cc renamed to test/in_memory_catalog_test.cc

@@ -17,7 +17,7 @@
  * under the License.
  */
 
-#include "iceberg/catalog/memory_catalog.h"
+#include "iceberg/catalog/in_memory_catalog.h"
 
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
@@ -100,7 +100,7 @@ TEST_F(InMemoryNamespaceTest, UnregisterTable) {
   const auto t = MakeTable(ns.levels, "t2");
   EXPECT_TRUE(root_namespace_.RegisterTable(t, "meta2"));
   EXPECT_TRUE(root_namespace_.UnregisterTable(t));
-  EXPECT_FALSE(root_namespace_.TableExists(t));
+  EXPECT_FALSE(root_namespace_.TableExists(t).value());
 }
 
 TEST_F(InMemoryNamespaceTest, RegisterTableOnNonExistingNamespaceFails) {
@@ -133,8 +133,9 @@ class MemoryCatalogTest : public ::testing::Test {
  protected:
   void SetUp() override {
     file_io_ = nullptr;  // TODO(Guotao): A real FileIO instance needs to be constructed.
-    catalog_ = std::make_unique<InMemoryCatalog>(file_io_, "/tmp/warehouse/");
-    catalog_->Initialize("test_catalog", {{"prop1", "val1"}});
+    std::unordered_map<std::string, std::string> properties = {{"prop1", "val1"}};
+    catalog_ = std::make_unique<InMemoryCatalog>("test_catalog", file_io_,
+                                                 "/tmp/warehouse/", properties);
   }
 
   std::shared_ptr<FileIO> file_io_;