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


##########
src/iceberg/catalog/rest/rest_catalog.cc:
##########
@@ -310,8 +340,22 @@ Status RestCatalog::RenameTable([[maybe_unused]] const 
TableIdentifier& from,
 }
 
 Result<std::shared_ptr<Table>> RestCatalog::LoadTable(
-    [[maybe_unused]] const TableIdentifier& identifier) {
-  return NotImplemented("Not implemented");
+    const TableIdentifier& identifier) const {
+  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));
+  // Cast away const since Table needs non-const Catalog pointer for mutations
+  auto non_const_catalog = 
std::const_pointer_cast<RestCatalog>(shared_from_this());
+  return Table::Make(identifier, load_result.metadata,
+                     std::move(load_result.metadata_location), file_io_,
+                     non_const_catalog);

Review Comment:
   ```suggestion
     return Table::Make(identifier, std::move(load_result.metadata),
                        std::move(load_result.metadata_location), file_io_,
                        std::move(non_const_catalog));
   ```



##########
src/iceberg/test/rest_catalog_test.cc:
##########
@@ -136,6 +136,30 @@ class RestCatalogIntegrationTest : public ::testing::Test {
     return RestCatalog::Make(*config, std::make_shared<test::StdFileIO>());
   }
 
+  // Helper function to create a default schema for testing
+  std::shared_ptr<Schema> CreateDefaultSchema() {
+    return std::make_shared<Schema>(
+        std::vector<SchemaField>{SchemaField::MakeRequired(1, "id", int32()),
+                                 SchemaField::MakeOptional(2, "data", 
string())},
+        /*schema_id=*/1);
+  }
+
+  // Helper function to create a default unpartitioned partition spec
+  std::shared_ptr<PartitionSpec> CreateDefaultPartitionSpec() {
+    auto partition_spec_result =
+        PartitionSpec::Make(PartitionSpec::kInitialSpecId, {}, 0);
+    EXPECT_THAT(partition_spec_result, IsOk());
+    return {std::move(*partition_spec_result)};

Review Comment:
   ```suggestion
       return PartitionSpec::Unpartitioned();
   ```



##########
src/iceberg/test/rest_catalog_test.cc:
##########
@@ -136,6 +136,30 @@ class RestCatalogIntegrationTest : public ::testing::Test {
     return RestCatalog::Make(*config, std::make_shared<test::StdFileIO>());
   }
 
+  // Helper function to create a default schema for testing
+  std::shared_ptr<Schema> CreateDefaultSchema() {
+    return std::make_shared<Schema>(
+        std::vector<SchemaField>{SchemaField::MakeRequired(1, "id", int32()),
+                                 SchemaField::MakeOptional(2, "data", 
string())},
+        /*schema_id=*/1);
+  }
+
+  // Helper function to create a default unpartitioned partition spec
+  std::shared_ptr<PartitionSpec> CreateDefaultPartitionSpec() {
+    auto partition_spec_result =
+        PartitionSpec::Make(PartitionSpec::kInitialSpecId, {}, 0);
+    EXPECT_THAT(partition_spec_result, IsOk());
+    return {std::move(*partition_spec_result)};
+  }
+
+  // Helper function to create a default unsorted sort order
+  std::shared_ptr<SortOrder> CreateDefaultSortOrder() {
+    auto sort_order_result =
+        SortOrder::Make(SortOrder::kUnsortedOrderId, std::vector<SortField>{});
+    EXPECT_THAT(sort_order_result, IsOk());
+    return {std::move(*sort_order_result)};

Review Comment:
   ```suggestion
       return SortOrder:Unsorted();
   ```



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to