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


##########
src/iceberg/catalog/in_memory_catalog.cc:
##########
@@ -441,44 +376,58 @@ Result<std::vector<TableIdentifier>> 
InMemoryCatalogImpl::ListTables(
   return table_idents;
 }
 
-Result<std::unique_ptr<Table>> InMemoryCatalogImpl::CreateTable(
+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 NotImplemented("create table");
 }
 
-Result<std::unique_ptr<Table>> InMemoryCatalogImpl::UpdateTable(
+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 NotImplemented("update table");
 }
 
-Result<std::shared_ptr<Transaction>> InMemoryCatalogImpl::StageCreateTable(
+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 NotImplemented("stage create table");
 }
 
-Result<bool> InMemoryCatalogImpl::TableExists(const TableIdentifier& 
identifier) const {
+Result<bool> InMemoryCatalog::TableExists(const TableIdentifier& identifier) 
const {
   std::unique_lock lock(mutex_);
   return root_namespace_->TableExists(identifier);
 }
 
-Status InMemoryCatalogImpl::DropTable(const TableIdentifier& identifier, bool 
purge) {
+Status 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);
 }
 
-Result<std::shared_ptr<Table>> InMemoryCatalogImpl::LoadTable(
+Result<std::shared_ptr<Table>> InMemoryCatalog::LoadTable(
     const TableIdentifier& identifier) const {
-  return NotImplemented("load table");
+  if (!file_io_) [[unlikely]] {
+    return NotSupported("file_io is not set for catalog {}", catalog_name_);
+  }
+
+  std::unique_lock lock(mutex_);

Review Comment:
   nit: add a block to restrict the lock scope to GetTableMetadataLocation.



-- 
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