wgtmac commented on code in PR #80:
URL: https://github.com/apache/iceberg-cpp/pull/80#discussion_r2101714110


##########
src/iceberg/catalog/in_memory_catalog.h:
##########
@@ -0,0 +1,99 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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 "iceberg/catalog.h"
+
+namespace iceberg {
+/**
+ * @brief An in-memory implementation of the Iceberg Catalog interface.
+ *
+ * This catalog stores all metadata purely in memory, with no persistence to 
disk
+ * or external systems. It is primarily intended for unit tests, prototyping, 
or
+ * demonstration purposes.
+ *
+ * @note This class is **not** suitable for production use.
+ *       All data will be lost when the process exits.
+ */

Review Comment:
   ```suggestion
    /// \brief An in-memory implementation of the Iceberg Catalog interface.
    ///
    /// This catalog stores all metadata purely in memory, with no persistence 
to disk
    /// or external systems. It is primarily intended for unit tests, 
prototyping, or
    /// demonstration purposes.
    ///
    /// \note This class is **not** suitable for production use.
    ///       All data will be lost when the process exits.
   ```



##########
src/iceberg/catalog.h:
##########
@@ -42,6 +43,67 @@ class ICEBERG_EXPORT Catalog {
   /// \brief Return the name for this catalog
   virtual std::string_view name() const = 0;
 
+  /// \brief Create a namespace with associated properties.
+  ///
+  /// \param ns the namespace to create
+  /// \param properties a key-value map of metadata for the namespace
+  /// \return Status::OK if created successfully;
+  ///         ErrorKind::kAlreadyExists if the namespace already exists;
+  ///         ErrorKind::kNotSupported if the operation is not supported
+  virtual Status CreateNamespace(
+      const Namespace& ns,
+      const std::unordered_map<std::string, std::string>& properties) = 0;
+
+  /// \brief List child namespaces from the given namespace.
+  ///
+  /// \param ns the parent namespace
+  /// \return a list of child namespaces;
+  ///         ErrorKind::kNoSuchNamespace if the given namespace does not exist
+  virtual Result<std::vector<Namespace>> ListNamespaces(const Namespace& ns) 
const = 0;
+
+  /// \brief Get metadata properties for a namespace.
+  ///
+  /// \param ns the namespace to look up
+  /// \return a key-value map of metadata properties;
+  ///         ErrorKind::kNoSuchNamespace if the namespace does not exist
+  virtual Result<std::unordered_map<std::string, std::string>> 
GetNamespaceProperties(
+      const Namespace& ns) const = 0;
+
+  /// \brief Drop a namespace.
+  ///
+  /// \param ns the namespace to drop
+  /// \return Status::OK if dropped successfully;
+  ///         ErrorKind::kNoSuchNamespace if the namespace does not exist;
+  ///         ErrorKind::kNotAllowed if the namespace is not empty
+  virtual Status DropNamespace(const Namespace& ns) = 0;
+
+  /// \brief Check whether the namespace exists.
+  ///
+  /// \param ns the namespace to check
+  /// \return true if the namespace exists, false otherwise
+  virtual Result<bool> NamespaceExists(const Namespace& ns) const = 0;
+
+  /// \brief Set metadata properties on a namespace.
+  ///
+  /// \param ns the namespace to modify
+  /// \param properties the properties to set or update
+  /// \return Status::OK if updated successfully;
+  ///         ErrorKind::kNoSuchNamespace if the namespace does not exist;
+  ///         ErrorKind::kNotSupported if the operation is not supported
+  virtual Status SetNamespaceProperties(

Review Comment:
   What about merging `SetNamespaceProperties` and `RemoveNamespaceProperties` 
into a single `UpdateNamespaceProperties(updates, removals)`? This is a single 
call in the rest catalog spec.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to