Skip to content

Conversation

@HeartLinked
Copy link
Contributor

@HeartLinked HeartLinked commented Dec 25, 2025

close #392 .
close #396 .
close #394 .

/// 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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the scenarios where the namespace does not exist or is not empty, return "NoSuchNamespace/NamespaceNotEmpty" and unify the return with Status. I think it is more suitable for the caller to handle it, and the Status can also include the specific namespace name. In scenarios with multiple nested calls, the outermost layer can more easily identify the root cause.

{
std::lock_guard guard(session_mutex_);
PrepareSession(path, headers);
PrepareSession(path, headers, params);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's reorder params and headers for PrepareSession as well?

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));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please reuse LoadTable instead of calling the client.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement LoadTable for REST catalog Implement TableExists for REST catalog Implement DropTable for REST catalog

3 participants