Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/iceberg/catalog.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ class ICEBERG_EXPORT Catalog {
/// \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;
virtual Result<bool> DropNamespace(const Namespace& ns) = 0;

/// \brief Check whether the namespace exists.
///
Expand Down Expand Up @@ -160,10 +160,10 @@ class ICEBERG_EXPORT Catalog {
///
/// \param identifier a table identifier
/// \param purge if true, delete all data and metadata files in the table
/// \return Status indicating the outcome of the operation.
/// \return Result<bool> indicating the outcome of the operation.
/// - On success, the table was dropped (or did not exist).
/// - On failure, contains error information.
virtual Status DropTable(const TableIdentifier& identifier, bool purge) = 0;
virtual Result<bool> DropTable(const TableIdentifier& identifier, bool purge) = 0;

/// \brief Rename a table
///
Expand Down
36 changes: 20 additions & 16 deletions src/iceberg/catalog/memory/in_memory_catalog.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,10 @@ class ICEBERG_EXPORT InMemoryNamespace {
/// \brief Deletes an existing namespace.
///
/// \param namespace_ident The namespace to delete.
/// \return Status::OK if the namespace is deleted;
/// ErrorKind::kNoSuchNamespace if the namespace does not exist;
/// ErrorKind::kNotAllowed if the namespace is not empty.
Status DropNamespace(const Namespace& namespace_ident);
/// \return Result<bool> indicating whether the namespace was deleted.
/// Returns false if the namespace does not exist.
/// Returns error if the namespace is not empty.
Result<bool> DropNamespace(const Namespace& namespace_ident);

/// \brief Retrieves the properties of the specified namespace.
///
Expand Down Expand Up @@ -107,9 +107,9 @@ class ICEBERG_EXPORT InMemoryNamespace {
/// \brief Unregisters a table from the specified namespace.
///
/// \param table_ident The identifier of the table to unregister.
/// \return Status::OK if the table is removed;
/// ErrorKind::kNoSuchTable if the table does not exist.
Status UnregisterTable(const TableIdentifier& table_ident);
/// \return Result<bool> indicating whether the table was unregistered.
/// Returns false if the table does not exist.
Result<bool> DropTable(const TableIdentifier& table_ident);

/// \brief Checks if a table exists in the specified namespace.
///
Expand Down Expand Up @@ -224,7 +224,7 @@ Status InMemoryNamespace::CreateNamespace(
return {};
}

Status InMemoryNamespace::DropNamespace(const Namespace& namespace_ident) {
Result<bool> InMemoryNamespace::DropNamespace(const Namespace& namespace_ident) {
if (namespace_ident.levels.empty()) {
return InvalidArgument("namespace identifier is empty");
}
Expand All @@ -238,7 +238,7 @@ Status InMemoryNamespace::DropNamespace(const Namespace& namespace_ident) {

const auto it = parentRs.value()->children_.find(to_delete);
if (it == parentRs.value()->children_.end()) {
return NotFound("namespace {} is not found", to_delete);
return false;
}

const auto& target = it->second;
Expand All @@ -247,7 +247,7 @@ Status InMemoryNamespace::DropNamespace(const Namespace& namespace_ident) {
}

parentRs.value()->children_.erase(to_delete);
return {};
return true;
}

Result<std::unordered_map<std::string, std::string>> InMemoryNamespace::GetProperties(
Expand Down Expand Up @@ -299,11 +299,15 @@ Status InMemoryNamespace::RegisterTable(const TableIdentifier& table_ident,
return {};
}

Status InMemoryNamespace::UnregisterTable(const TableIdentifier& table_ident) {
Result<bool> InMemoryNamespace::DropTable(const TableIdentifier& table_ident) {
const auto ns = GetNamespace(this, table_ident.ns);
ICEBERG_RETURN_UNEXPECTED(ns);
ns.value()->table_metadata_locations_.erase(table_ident.name);
return {};
const auto it = ns.value()->table_metadata_locations_.find(table_ident.name);
if (it == ns.value()->table_metadata_locations_.end()) {
return false;
}
ns.value()->table_metadata_locations_.erase(it);
return true;
}

Result<bool> InMemoryNamespace::TableExists(const TableIdentifier& table_ident) const {
Expand Down Expand Up @@ -369,7 +373,7 @@ Result<std::vector<Namespace>> InMemoryCatalog::ListNamespaces(
return root_namespace_->ListNamespaces(ns);
}

Status InMemoryCatalog::DropNamespace(const Namespace& ns) {
Result<bool> InMemoryCatalog::DropNamespace(const Namespace& ns) {
std::unique_lock lock(mutex_);
return root_namespace_->DropNamespace(ns);
}
Expand Down Expand Up @@ -453,10 +457,10 @@ Result<bool> InMemoryCatalog::TableExists(const TableIdentifier& identifier) con
return root_namespace_->TableExists(identifier);
}

Status InMemoryCatalog::DropTable(const TableIdentifier& identifier, bool purge) {
Result<bool> InMemoryCatalog::DropTable(const TableIdentifier& identifier, bool purge) {
std::unique_lock lock(mutex_);
// TODO(Guotao): Delete all metadata files if purge is true.
return root_namespace_->UnregisterTable(identifier);
return root_namespace_->DropTable(identifier);
}

Status InMemoryCatalog::RenameTable(const TableIdentifier& from,
Expand Down
4 changes: 2 additions & 2 deletions src/iceberg/catalog/memory/in_memory_catalog.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class ICEBERG_EXPORT InMemoryCatalog

Result<std::vector<Namespace>> ListNamespaces(const Namespace& ns) const override;

Status DropNamespace(const Namespace& ns) override;
Result<bool> DropNamespace(const Namespace& ns) override;

Result<bool> NamespaceExists(const Namespace& ns) const override;

Expand Down Expand Up @@ -89,7 +89,7 @@ class ICEBERG_EXPORT InMemoryCatalog

Result<bool> TableExists(const TableIdentifier& identifier) const override;

Status DropTable(const TableIdentifier& identifier, bool purge) override;
Result<bool> DropTable(const TableIdentifier& identifier, bool purge) override;

Status RenameTable(const TableIdentifier& from, const TableIdentifier& to) override;

Expand Down
5 changes: 3 additions & 2 deletions src/iceberg/catalog/rest/http_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -240,12 +240,13 @@ Result<HttpResponse> HttpClient::Head(
}

Result<HttpResponse> HttpClient::Delete(
const std::string& path, const std::unordered_map<std::string, std::string>& headers,
const std::string& path, const std::unordered_map<std::string, std::string>& params,
const std::unordered_map<std::string, std::string>& headers,
const ErrorHandler& error_handler) {
cpr::Response response;
{
std::lock_guard guard(session_mutex_);
PrepareSession(path, headers);
PrepareSession(path, headers, params);
response = session_->Delete();
}

Expand Down
1 change: 1 addition & 0 deletions src/iceberg/catalog/rest/http_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ class ICEBERG_REST_EXPORT HttpClient {

/// \brief Sends a DELETE request.
Result<HttpResponse> Delete(const std::string& path,
const std::unordered_map<std::string, std::string>& params,
const std::unordered_map<std::string, std::string>& headers,
const ErrorHandler& error_handler);

Expand Down
90 changes: 74 additions & 16 deletions src/iceberg/catalog/rest/rest_catalog.cc
Original file line number Diff line number Diff line change
Expand Up @@ -185,14 +185,22 @@ Result<std::unordered_map<std::string, std::string>> RestCatalog::GetNamespacePr
return get_response.properties;
}

Status RestCatalog::DropNamespace(const Namespace& ns) {
Result<bool> RestCatalog::DropNamespace(const Namespace& ns) {
ICEBERG_RETURN_UNEXPECTED(
CheckEndpoint(supported_endpoints_, Endpoint::DropNamespace()));
ICEBERG_ASSIGN_OR_RAISE(auto path, paths_->Namespace_(ns));
ICEBERG_ASSIGN_OR_RAISE(
const auto response,
client_->Delete(path, /*headers=*/{}, *DropNamespaceErrorHandler::Instance()));
return {};

auto response_or_error = client_->Delete(path, /*params=*/{}, /*headers=*/{},
*DropNamespaceErrorHandler::Instance());

if (!response_or_error.has_value()) {
// Catch NoSuchNamespaceException and return false
if (response_or_error.error().kind == ErrorKind::kNoSuchNamespace) {
return false;
}
ICEBERG_RETURN_UNEXPECTED(response_or_error);
}
return true;
}

Result<bool> RestCatalog::NamespaceExists(const Namespace& ns) const {
Expand All @@ -212,9 +220,8 @@ Result<bool> RestCatalog::NamespaceExists(const Namespace& ns) const {
auto response_or_error =
client_->Head(path, /*headers=*/{}, *NamespaceErrorHandler::Instance());
if (!response_or_error.has_value()) {
const auto& error = response_or_error.error();
// catch NoSuchNamespaceException/404 and return false
if (error.kind == ErrorKind::kNoSuchNamespace) {
if (response_or_error.error().kind == ErrorKind::kNoSuchNamespace) {
return false;
}
ICEBERG_RETURN_UNEXPECTED(response_or_error);
Expand Down Expand Up @@ -294,24 +301,75 @@ Result<std::shared_ptr<Transaction>> RestCatalog::StageCreateTable(
return NotImplemented("Not implemented");
}

Status RestCatalog::DropTable([[maybe_unused]] const TableIdentifier& identifier,
[[maybe_unused]] bool purge) {
return NotImplemented("Not implemented");
Result<bool> RestCatalog::DropTable(const TableIdentifier& identifier, bool purge) {
ICEBERG_RETURN_UNEXPECTED(CheckEndpoint(supported_endpoints_, Endpoint::DeleteTable()));
ICEBERG_ASSIGN_OR_RAISE(auto path, paths_->Table(identifier));

std::unordered_map<std::string, std::string> params;
if (purge) {
params["purgeRequested"] = "true";
}
auto response_or_error =
client_->Delete(path, params, /*headers=*/{}, *TableErrorHandler::Instance());
if (!response_or_error.has_value()) {
const auto& error = response_or_error.error();
if (error.kind == ErrorKind::kNoSuchTable) {
return false;
}
ICEBERG_RETURN_UNEXPECTED(response_or_error);
}
return true;
}

Result<bool> RestCatalog::TableExists(
[[maybe_unused]] const TableIdentifier& identifier) const {
return NotImplemented("Not implemented");
Result<bool> RestCatalog::TableExists(const TableIdentifier& identifier) const {
auto check = CheckEndpoint(supported_endpoints_, Endpoint::TableExists());
if (!check.has_value()) {
// Fall back to LoadTable endpoint (GET)
ICEBERG_ASSIGN_OR_RAISE(auto path, paths_->Table(identifier));
auto response_or_error =
client_->Get(path, /*params=*/{}, /*headers=*/{}, *TableErrorHandler::Instance());
if (!response_or_error.has_value()) {
if (response_or_error.error().kind == ErrorKind::kNoSuchTable) {
return false;
}
ICEBERG_RETURN_UNEXPECTED(response_or_error);
}
// GET succeeded, table exists
return true;
}

ICEBERG_ASSIGN_OR_RAISE(auto path, paths_->Table(identifier));
auto response_or_error =
client_->Head(path, /*headers=*/{}, *TableErrorHandler::Instance());
if (!response_or_error.has_value()) {
// catch NoSuchTableException/404 and return false
if (response_or_error.error().kind == ErrorKind::kNoSuchTable) {
return false;
}
ICEBERG_RETURN_UNEXPECTED(response_or_error);
}
return true;
}

Status RestCatalog::RenameTable([[maybe_unused]] const TableIdentifier& from,
[[maybe_unused]] const TableIdentifier& to) {
return NotImplemented("Not implemented");
}

Result<std::shared_ptr<Table>> RestCatalog::LoadTable(
[[maybe_unused]] const TableIdentifier& identifier) {
return NotImplemented("Not implemented");
Result<std::shared_ptr<Table>> RestCatalog::LoadTable(const TableIdentifier& identifier) {
ICEBERG_RETURN_UNEXPECTED(CheckEndpoint(supported_endpoints_, Endpoint::LoadTable()));
ICEBERG_ASSIGN_OR_RAISE(auto path, paths_->Table(identifier));

ICEBERG_ASSIGN_OR_RAISE(
const auto response,
client_->Get(path, /*params=*/{}, /*headers=*/{}, *TableErrorHandler::Instance()));

// TODO(Feiyang Li): support load metadata table
ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.body()));
ICEBERG_ASSIGN_OR_RAISE(auto load_result, LoadTableResultFromJson(json));
return Table::Make(identifier, load_result.metadata,
std::move(load_result.metadata_location), file_io_,
shared_from_this());
}

Result<std::shared_ptr<Table>> RestCatalog::RegisterTable(
Expand Down
4 changes: 2 additions & 2 deletions src/iceberg/catalog/rest/rest_catalog.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class ICEBERG_REST_EXPORT RestCatalog : public Catalog,
Result<std::unordered_map<std::string, std::string>> GetNamespaceProperties(
const Namespace& ns) const override;

Status DropNamespace(const Namespace& ns) override;
Result<bool> DropNamespace(const Namespace& ns) override;

Result<bool> NamespaceExists(const Namespace& ns) const override;

Expand Down Expand Up @@ -95,7 +95,7 @@ class ICEBERG_REST_EXPORT RestCatalog : public Catalog,

Status RenameTable(const TableIdentifier& from, const TableIdentifier& to) override;

Status DropTable(const TableIdentifier& identifier, bool purge) override;
Result<bool> DropTable(const TableIdentifier& identifier, bool purge) override;

Result<std::shared_ptr<Table>> LoadTable(const TableIdentifier& identifier) override;

Expand Down
4 changes: 2 additions & 2 deletions src/iceberg/test/mock_catalog.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class MockCatalog : public Catalog {
(const std::unordered_set<std::string>&)),
(override));

MOCK_METHOD(Status, DropNamespace, (const Namespace&), (override));
MOCK_METHOD(Result<bool>, DropNamespace, (const Namespace&), (override));

MOCK_METHOD(Result<bool>, NamespaceExists, (const Namespace&), (const, override));

Expand All @@ -75,7 +75,7 @@ class MockCatalog : public Catalog {

MOCK_METHOD(Result<bool>, TableExists, (const TableIdentifier&), (const, override));

MOCK_METHOD(Status, DropTable, (const TableIdentifier&, bool), (override));
MOCK_METHOD(Result<bool>, DropTable, (const TableIdentifier&, bool), (override));

MOCK_METHOD(Status, RenameTable, (const TableIdentifier&, const TableIdentifier&),
(override));
Expand Down
Loading
Loading