Skip to content

Commit 99675e4

Browse files
gty404gty404
committed
Change "set/remove namespace properties" to "update namespace properties"
1 parent 1831078 commit 99675e4

File tree

4 files changed

+117
-145
lines changed

4 files changed

+117
-145
lines changed

‎src/iceberg/catalog.h‎

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -83,26 +83,17 @@ class ICEBERG_EXPORT Catalog {
8383
/// \return true if the namespace exists, false otherwise
8484
virtual Result<bool> NamespaceExists(const Namespace& ns) const = 0;
8585

86-
/// \brief Set metadata properties on a namespace.
86+
/// \brief Update a namespace's properties by applying additions and removals.
8787
///
88-
/// \param ns the namespace to modify
89-
/// \param properties the properties to set or update
90-
/// \return Status::OK if updated successfully;
88+
/// \param ns the namespace to update
89+
/// \param updates a set of properties to add or overwrite
90+
/// \param removals a set of property keys to remove
91+
/// \return Status::OK if the update is successful;
9192
/// ErrorKind::kNoSuchNamespace if the namespace does not exist;
92-
/// ErrorKind::kNotSupported if the operation is not supported
93-
virtual Status SetNamespaceProperties(
94-
const Namespace& ns,
95-
const std::unordered_map<std::string, std::string>& properties) = 0;
96-
97-
/// \brief Remove a set of metadata properties from a namespace.
98-
///
99-
/// \param ns the namespace to modify
100-
/// \param properties the set of property keys to remove
101-
/// \return Status::OK if removed successfully;
102-
/// ErrorKind::kNoSuchNamespace if the namespace does not exist;
103-
/// ErrorKind::kNotSupported if the operation is not supported
104-
virtual Status RemoveNamespaceProperties(
105-
const Namespace& ns, const std::unordered_set<std::string>& properties) = 0;
93+
/// ErrorKind::kUnsupported if the operation is not supported
94+
virtual Status UpdateNamespaceProperties(
95+
const Namespace& ns, const std::unordered_map<std::string, std::string>& updates,
96+
const std::unordered_set<std::string>& removals) = 0;
10697

10798
/// \brief Return all the identifiers under this namespace
10899
///

‎src/iceberg/catalog/in_memory_catalog.cc‎

Lines changed: 93 additions & 117 deletions
Original file line numberDiff line numberDiff line change
@@ -33,113 +33,104 @@ namespace iceberg {
3333

3434
namespace {
3535

36-
/**
37-
* \brief A hierarchical namespace that manages namespaces and table metadata in-memory.
38-
*
39-
* Each InMemoryNamespace represents a namespace level and can contain properties,
40-
* tables, and child namespaces. This structure enables a tree-like representation
41-
* of nested namespaces.
42-
*/
36+
/// \brief A hierarchical namespace that manages namespaces and table metadata in-memory.
37+
///
38+
/// Each InMemoryNamespace represents a namespace level and can contain properties,
39+
/// tables, and child namespaces. This structure enables a tree-like representation
40+
/// of nested namespaces.
4341
class ICEBERG_EXPORT InMemoryNamespace {
4442
public:
45-
/**
46-
* \brief Checks whether the given namespace exists.
47-
* \param[in] namespace_ident The namespace to check.
48-
* \return Status indicating success or failure.
49-
*/
43+
/// \brief Checks whether the given namespace exists.
44+
///
45+
/// \param namespace_ident The namespace to check.
46+
/// \return Result<bool> indicating whether the namespace exists.
5047
Result<bool> NamespaceExists(const Namespace& namespace_ident) const;
5148

52-
/**
53-
* \brief Lists immediate child namespaces under the given parent namespace.
54-
* \param[in] parent_namespace_ident The optional parent namespace.
55-
* \return A vector of child namespaces.
56-
*/
49+
/// \brief Lists immediate child namespaces under the given parent namespace.
50+
///
51+
/// \param parent_namespace_ident The parent namespace to list children for.
52+
/// \return A vector of child namespaces if found; error otherwise.
5753
Result<std::vector<Namespace>> ListNamespaces(
5854
const Namespace& parent_namespace_ident) const;
5955

60-
/**
61-
* \brief Creates a new namespace with the specified properties.
62-
* \param[in] namespace_ident The namespace to create.
63-
* \param[in] properties A map of key-value pairs to associate with the namespace.
64-
* \return Status indicating success or failure.
65-
*/
56+
/// \brief Creates a new namespace with the specified properties.
57+
///
58+
/// \param namespace_ident The namespace to create.
59+
/// \param properties A map of key-value pairs to associate with the namespace.
60+
/// \return Status::OK if the namespace is created;
61+
/// ErrorKind::kAlreadyExists if the namespace already exists.
6662
Status CreateNamespace(const Namespace& namespace_ident,
6763
const std::unordered_map<std::string, std::string>& properties);
6864

69-
/**
70-
* \brief Deletes an existing namespace.
71-
* \param[in] namespace_ident The namespace to delete.
72-
* \return Status indicating success or failure.
73-
*/
65+
/// \brief Deletes an existing namespace.
66+
///
67+
/// \param namespace_ident The namespace to delete.
68+
/// \return Status::OK if the namespace is deleted;
69+
/// ErrorKind::kNoSuchNamespace if the namespace does not exist;
70+
/// ErrorKind::kNotAllowed if the namespace is not empty.
7471
Status DropNamespace(const Namespace& namespace_ident);
7572

76-
/**
77-
* \brief Retrieves the properties of the specified namespace.
78-
* \param[in] namespace_ident The namespace whose properties to retrieve.
79-
* \return An containing the properties map if the namespace exists;
80-
* Errpr otherwise.
81-
*/
73+
/// \brief Retrieves the properties of the specified namespace.
74+
///
75+
/// \param namespace_ident The namespace whose properties are to be retrieved.
76+
/// \return A map of property key-value pairs if the namespace exists;
77+
/// error otherwise.
8278
Result<std::unordered_map<std::string, std::string>> GetProperties(
8379
const Namespace& namespace_ident) const;
8480

85-
/**
86-
* \brief Replaces all properties of the given namespace.
87-
* \param[in] namespace_ident The namespace whose properties will be replaced.
88-
* \param[in] properties The new properties map.
89-
* \return Status indicating success or failure.
90-
*/
91-
Status SetNamespaceProperties(
81+
/// \brief Updates a namespace's properties by applying additions and removals.
82+
///
83+
/// \param namespace_ident The namespace to update.
84+
/// \param updates Properties to add or overwrite.
85+
/// \param removals Property keys to remove.
86+
/// \return Status::OK if the update is successful;
87+
/// ErrorKind::kNoSuchNamespace if the namespace does not exist;
88+
/// ErrorKind::kUnsupported if the operation is not supported.
89+
Status UpdateNamespaceProperties(
9290
const Namespace& namespace_ident,
93-
const std::unordered_map<std::string, std::string>& properties);
91+
const std::unordered_map<std::string, std::string>& updates,
92+
const std::unordered_set<std::string>& removals);
9493

95-
/// \brief Remove a set of metadata properties from a namespace.
94+
/// \brief Lists all table names under the specified namespace.
9695
///
97-
/// \param namespace_ident the namespace to modify
98-
/// \param properties the set of property keys to remove
99-
/// \return Status::OK if removed successfully;
100-
/// ErrorKind::kNoSuchNamespace if the namespace does not exist;
101-
/// ErrorKind::kNotSupported if the operation is not supported
102-
Status RemoveNamespaceProperties(const Namespace& namespace_ident,
103-
const std::unordered_set<std::string>& properties);
104-
105-
/**
106-
* \brief Lists all table names under the specified namespace.
107-
* \param[in] namespace_ident The namespace from which to list tables.
108-
* \return A vector of table names or error.
109-
*/
96+
/// \param namespace_ident The namespace to list tables from.
97+
/// \return A vector of table names if successful; error otherwise.
11098
Result<std::vector<std::string>> ListTables(const Namespace& namespace_ident) const;
11199

112-
/**
113-
* \brief Registers a table in the given namespace with a metadata location.
114-
* \param[in] table_ident The fully qualified identifier of the table.
115-
* \param[in] metadata_location The path to the table's metadata.
116-
* \return Status indicating success or failure.
117-
*/
118-
Status RegisterTable(TableIdentifier const& table_ident,
100+
/// \brief Registers a table in the given namespace with a metadata location.
101+
///
102+
/// \param table_ident The fully qualified identifier of the table.
103+
/// \param metadata_location The path to the table's metadata.
104+
/// \return Status::OK if the table is registered;
105+
/// Error otherwise.
106+
Status RegisterTable(const TableIdentifier& table_ident,
119107
const std::string& metadata_location);
120108

121-
/**
122-
* \brief Unregisters a table from the specified namespace.
123-
* \param[in] table_ident The identifier of the table to unregister.
124-
* \return Status indicating success or failure.
125-
*/
126-
Status UnregisterTable(TableIdentifier const& table_ident);
127-
128-
/**
129-
* \brief Checks if a table exists in the specified namespace.
130-
* \param[in] table_ident The identifier of the table to check.
131-
* \return Result<bool> indicating table exists or not.
132-
*/
133-
Result<bool> TableExists(TableIdentifier const& table_ident) const;
134-
135-
/**
136-
* \brief Gets the metadata location for the specified table.
137-
* \param[in] table_ident The identifier of the table.
138-
* \return An string containing the metadata location if the table exists;
139-
* Error otherwise.
140-
*/
141-
Result<std::string> GetTableMetadataLocation(TableIdentifier const& table_ident) const;
109+
/// \brief Unregisters a table from the specified namespace.
110+
///
111+
/// \param table_ident The identifier of the table to unregister.
112+
/// \return Status::OK if the table is removed;
113+
/// ErrorKind::kNoSuchTable if the table does not exist.
114+
Status UnregisterTable(const TableIdentifier& table_ident);
115+
116+
/// \brief Checks if a table exists in the specified namespace.
117+
///
118+
/// \param table_ident The identifier of the table to check.
119+
/// \return Result<bool> indicating whether the table exists.
120+
Result<bool> TableExists(const TableIdentifier& table_ident) const;
121+
122+
/// \brief Gets the metadata location for the specified table.
123+
///
124+
/// \param table_ident The identifier of the table.
125+
/// \return The metadata location if the table exists; error otherwise.
126+
Result<std::string> GetTableMetadataLocation(const TableIdentifier& table_ident) const;
142127

128+
/// \brief Internal utility for retrieving a namespace node pointer from the tree.
129+
///
130+
/// \tparam NamespacePtr The type of the namespace node pointer.
131+
/// \param root The root namespace node.
132+
/// \param namespace_ident The fully qualified namespace to resolve.
133+
/// \return A pointer to the namespace node if it exists; error otherwise.
143134
template <typename NamespacePtr>
144135
static Result<NamespacePtr> GetNamespaceImpl(NamespacePtr root,
145136
const Namespace& namespace_ident) {
@@ -261,20 +252,17 @@ Result<std::unordered_map<std::string, std::string>> InMemoryNamespace::GetPrope
261252
return ns.value()->properties_;
262253
}
263254

264-
Status InMemoryNamespace::SetNamespaceProperties(
255+
Status InMemoryNamespace::UpdateNamespaceProperties(
265256
const Namespace& namespace_ident,
266-
const std::unordered_map<std::string, std::string>& properties) {
257+
const std::unordered_map<std::string, std::string>& updates,
258+
const std::unordered_set<std::string>& removals) {
267259
const auto ns = GetNamespace(this, namespace_ident);
268260
ICEBERG_RETURN_UNEXPECTED(ns);
269-
ns.value()->properties_ = properties;
270-
return {};
271-
}
272261

273-
Status InMemoryNamespace::RemoveNamespaceProperties(
274-
const Namespace& namespace_ident, const std::unordered_set<std::string>& properties) {
275-
const auto ns = GetNamespace(this, namespace_ident);
276-
ICEBERG_RETURN_UNEXPECTED(ns);
277-
std::ranges::for_each(properties,
262+
std::ranges::for_each(updates, [&](const auto& prop) {
263+
ns.value()->properties_[prop.first] = prop.second;
264+
});
265+
std::ranges::for_each(removals,
278266
[&](const auto& prop) { ns.value()->properties_.erase(prop); });
279267
return {};
280268
}
@@ -352,12 +340,9 @@ class ICEBERG_EXPORT InMemoryCatalogImpl {
352340
Result<std::unordered_map<std::string, std::string>> GetNamespaceProperties(
353341
const Namespace& ns) const;
354342

355-
Status SetNamespaceProperties(
356-
const Namespace& ns,
357-
const std::unordered_map<std::string, std::string>& properties);
358-
359-
Status RemoveNamespaceProperties(const Namespace& ns,
360-
const std::unordered_set<std::string>& properties);
343+
Status UpdateNamespaceProperties(
344+
const Namespace& ns, const std::unordered_map<std::string, std::string>& updates,
345+
const std::unordered_set<std::string>& removals);
361346

362347
Result<std::vector<TableIdentifier>> ListTables(const Namespace& ns) const;
363348

@@ -436,16 +421,11 @@ InMemoryCatalogImpl::GetNamespaceProperties(const Namespace& ns) const {
436421
return root_namespace_->GetProperties(ns);
437422
}
438423

439-
Status InMemoryCatalogImpl::SetNamespaceProperties(
440-
const Namespace& ns, const std::unordered_map<std::string, std::string>& properties) {
441-
std::unique_lock lock(mutex_);
442-
return root_namespace_->SetNamespaceProperties(ns, properties);
443-
}
444-
445-
Status InMemoryCatalogImpl::RemoveNamespaceProperties(
446-
const Namespace& ns, const std::unordered_set<std::string>& properties) {
424+
Status InMemoryCatalogImpl::UpdateNamespaceProperties(
425+
const Namespace& ns, const std::unordered_map<std::string, std::string>& updates,
426+
const std::unordered_set<std::string>& removals) {
447427
std::unique_lock lock(mutex_);
448-
return root_namespace_->RemoveNamespaceProperties(ns, properties);
428+
return root_namespace_->UpdateNamespaceProperties(ns, updates, removals);
449429
}
450430

451431
Result<std::vector<TableIdentifier>> InMemoryCatalogImpl::ListTables(
@@ -549,14 +529,10 @@ Result<bool> InMemoryCatalog::NamespaceExists(const Namespace& ns) const {
549529
return impl_->NamespaceExists(ns);
550530
}
551531

552-
Status InMemoryCatalog::RemoveNamespaceProperties(
553-
const Namespace& ns, const std::unordered_set<std::string>& properties) {
554-
return impl_->RemoveNamespaceProperties(ns, properties);
555-
}
556-
557-
Status InMemoryCatalog::SetNamespaceProperties(
558-
const Namespace& ns, const std::unordered_map<std::string, std::string>& properties) {
559-
return impl_->SetNamespaceProperties(ns, properties);
532+
Status InMemoryCatalog::UpdateNamespaceProperties(
533+
const Namespace& ns, const std::unordered_map<std::string, std::string>& updates,
534+
const std::unordered_set<std::string>& removals) {
535+
return impl_->UpdateNamespaceProperties(ns, updates, removals);
560536
}
561537

562538
Result<std::vector<TableIdentifier>> InMemoryCatalog::ListTables(

‎src/iceberg/catalog/in_memory_catalog.h‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -54,12 +54,9 @@ class ICEBERG_EXPORT InMemoryCatalog : public Catalog {
5454
Result<std::unordered_map<std::string, std::string>> GetNamespaceProperties(
5555
const Namespace& ns) const override;
5656

57-
Status SetNamespaceProperties(
58-
const Namespace& ns,
59-
const std::unordered_map<std::string, std::string>& properties) override;
60-
61-
Status RemoveNamespaceProperties(
62-
const Namespace& ns, const std::unordered_set<std::string>& properties) override;
57+
Status UpdateNamespaceProperties(
58+
const Namespace& ns, const std::unordered_map<std::string, std::string>& updates,
59+
const std::unordered_set<std::string>& removals) override;
6360

6461
Result<std::vector<TableIdentifier>> ListTables(const Namespace& ns) const override;
6562

‎test/in_memory_catalog_test.cc‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,8 @@ TEST_F(InMemoryCatalogTest, DropTable) {
6666

6767
TEST_F(InMemoryCatalogTest, Namespace) {
6868
Namespace ns{.levels = {"n1", "n2"}};
69-
std::unordered_map<std::string, std::string> properties = {{"prop1", "val1"}};
69+
std::unordered_map<std::string, std::string> properties = {{"prop1", "val1"},
70+
{"prop2", "val2"}};
7071
EXPECT_THAT(catalog_->CreateNamespace(ns, properties), IsOk());
7172
EXPECT_THAT(catalog_->CreateNamespace(ns, properties),
7273
IsError(ErrorKind::kAlreadyExists));
@@ -82,11 +83,18 @@ TEST_F(InMemoryCatalogTest, Namespace) {
8283

8384
auto propsRs = catalog_->GetNamespaceProperties(ns);
8485
EXPECT_THAT(propsRs, IsOk());
85-
ASSERT_EQ(propsRs->size(), 1U);
86+
ASSERT_EQ(propsRs->size(), 2U);
8687
ASSERT_EQ(propsRs.value().at("prop1"), "val1");
88+
ASSERT_EQ(propsRs.value().at("prop2"), "val2");
8789

88-
EXPECT_THAT(catalog_->SetNamespaceProperties(ns, {{"prop2", "val2"}}), IsOk());
89-
EXPECT_THAT(catalog_->RemoveNamespaceProperties(ns, {"prop2"}), IsOk());
90+
EXPECT_THAT(catalog_->UpdateNamespaceProperties(
91+
ns, {{"prop2", "val2-updated"}, {"prop3", "val3"}}, {"prop1"}),
92+
IsOk());
93+
propsRs = catalog_->GetNamespaceProperties(ns);
94+
EXPECT_THAT(propsRs, IsOk());
95+
ASSERT_EQ(propsRs->size(), 2U);
96+
ASSERT_EQ(propsRs.value().at("prop2"), "val2-updated");
97+
ASSERT_EQ(propsRs.value().at("prop3"), "val3");
9098
}
9199

92100
} // namespace iceberg

0 commit comments

Comments
 (0)