feat: Refresh method support for table

Author: shuxu.li

src/iceberg/catalog/in_memory_catalog.cc

@@ -466,10 +466,4 @@ std::unique_ptr<TableBuilder> InMemoryCatalog::BuildTable(
   throw IcebergError("not implemented");
 }
 
-Status InMemoryCatalog::UpdateTableMetaLocationInternal(
-    const TableIdentifier& identifier, const std::string& metadata_location) {
-  std::unique_lock lock(mutex_);
-  return root_namespace_->UpdateTableMetaLocation(identifier, metadata_location);
-}
-
 }  // namespace iceberg

src/iceberg/catalog/in_memory_catalog.h

@@ -84,7 +84,8 @@ class ICEBERG_EXPORT InMemoryCatalog
 
   Status DropTable(const TableIdentifier& identifier, bool purge) override;
 
-  Result<std::unique_ptr<Table>> LoadTable(const TableIdentifier& identifier) override;
+  virtual Result<std::unique_ptr<Table>> LoadTable(
+      const TableIdentifier& identifier) override;
 
   Result<std::shared_ptr<Table>> RegisterTable(
       const TableIdentifier& identifier,
@@ -93,10 +94,6 @@ class ICEBERG_EXPORT InMemoryCatalog
   std::unique_ptr<iceberg::TableBuilder> BuildTable(const TableIdentifier& identifier,
                                                     const Schema& schema) const override;
 
- protected:
-  Status UpdateTableMetaLocationInternal(const TableIdentifier& identifier,
-                                         const std::string& metadata_location);
-
  private:
   std::string catalog_name_;
   std::unordered_map<std::string, std::string> properties_;

src/iceberg/table.cc

@@ -35,7 +35,7 @@ const std::string& Table::uuid() const { return metadata_->table_uuid; }
 
 Status Table::Refresh() {
   if (!catalog_) {
-    return InvalidArgument("Refresh is not supported for table without a catalog");
+    return NotSupported("Refresh is not supported for table without a catalog");
   }
 
   ICEBERG_ASSIGN_OR_RAISE(auto refreshed_table, catalog_->LoadTable(identifier_));

test/in_memory_catalog_test.cc

@@ -117,32 +117,38 @@ TEST_F(InMemoryCatalogTest, RegisterTable) {
 TEST_F(InMemoryCatalogTest, RefreshTable) {
   TableIdentifier tableIdent{.ns = {}, .name = "t1"};
 
-  std::unique_ptr<TableMetadata> metadata;
-  ASSERT_NO_FATAL_FAILURE(ReadTableMetadata("TableMetadataV2Valid.json", &metadata));
-
-  metadata->current_snapshot_id = 3051729675574597004;
   auto table_location = GenerateTestTableLocation(tableIdent.name);
-  auto metadata_location = std::format("{}v0.metadata.json", table_location);
-  auto status = TableMetadataUtil::Write(*file_io_, metadata_location, *metadata);
-  EXPECT_THAT(status, IsOk());
-
+  auto buildTable = [this, &tableIdent, &table_location](
+                        int64_t snapshot_id, int64_t version) -> std::unique_ptr<Table> {
+    std::unique_ptr<TableMetadata> metadata;
+
+    ReadTableMetadata("TableMetadataV2Valid.json", &metadata);
+    metadata->current_snapshot_id = snapshot_id;
+
+    auto metadata_location = std::format("{}v{}.metadata.json", table_location, version);
+    auto status = TableMetadataUtil::Write(*file_io_, metadata_location, *metadata);
+    EXPECT_THAT(status, IsOk());
+
+    return std::make_unique<Table>(tableIdent, std::move(metadata), metadata_location,
+                                   file_io_, std::static_pointer_cast<Catalog>(catalog_));
+  };
+
+  auto table_v0 = buildTable(3051729675574597004, 0);
+  auto table_v1 = buildTable(3055729675574597004, 1);
+  EXPECT_CALL(*catalog_, LoadTable(::testing::_))
+      .WillOnce(::testing::Return(
+          ::testing::ByMove(Result<std::unique_ptr<Table>>(std::move(table_v0)))))
+      .WillOnce(::testing::Return(
+          ::testing::ByMove(Result<std::unique_ptr<Table>>(std::move(table_v1)))));
+
+  auto metadata_location = std::format("{}v{}.metadata.json", table_location, 0);
   auto table = catalog_->RegisterTable(tableIdent, metadata_location);
   EXPECT_THAT(table, IsOk());
   ASSERT_TRUE(table.value()->current_snapshot().has_value());
   ASSERT_EQ(table.value()->current_snapshot().value()->snapshot_id, 3051729675574597004);
 
-  // update table version to 3055729675574597004
-  metadata_location = std::format("{}v1.metadata.json", table_location);
-  metadata->current_snapshot_id = 3055729675574597004;
-  status = TableMetadataUtil::Write(*file_io_, metadata_location, *metadata);
-  EXPECT_THAT(status, IsOk());
-
-  // update table metadata location in catalog
-  status = catalog_->UpdateTableMetaLocation(tableIdent, metadata_location);
-  EXPECT_THAT(status, IsOk());
-
   // same version
-  status = table.value()->Refresh();
+  auto status = table.value()->Refresh();
   EXPECT_THAT(status, IsOk());
   ASSERT_TRUE(table.value()->current_snapshot().has_value());
   ASSERT_EQ(table.value()->current_snapshot().value()->snapshot_id, 3055729675574597004);

test/mock_catalog.h

@@ -30,11 +30,7 @@ class ICEBERG_EXPORT MockInMemoryCatalog : public InMemoryCatalog {
                       std::unordered_map<std::string, std::string> const& properties)
       : InMemoryCatalog(name, file_io, warehouse_location, properties) {}
 
-  /// \brief Directly sets the metadata location of a table.
-  /// \note This should only be used in unit tests.
-  Status UpdateTableMetaLocation(const TableIdentifier& identifier,
-                                 const std::string& metadata_location) {
-    return UpdateTableMetaLocationInternal(identifier, metadata_location);
-  }
+  MOCK_METHOD(Result<std::unique_ptr<Table>>, LoadTable, (const TableIdentifier&),
+              (override));
 };
 }  // namespace iceberg