Skip to content

Commit 1777794

Browse files
authored
feat: implement RenameTable in InMemoryCatalog (#742)
Replace the NotImplemented stub with a full implementation that: - Returns early as a no-op when renaming a table to itself - Returns NoSuchTable when the source table does not exist - Returns AlreadyExists when the destination table already exists - Moves the metadata location from source to destination identifier Add unit test covering rename to self, nonexistent source, destination conflict, and full rename + load verification.
1 parent 64691e4 commit 1777794

2 files changed

Lines changed: 72 additions & 1 deletion

File tree

src/iceberg/catalog/memory/in_memory_catalog.cc

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -518,7 +518,35 @@ Status InMemoryCatalog::DropTable(const TableIdentifier& identifier, bool purge)
518518
Status InMemoryCatalog::RenameTable(const TableIdentifier& from,
519519
const TableIdentifier& to) {
520520
std::unique_lock lock(mutex_);
521-
return NotImplemented("rename table");
521+
522+
// Renaming a table to itself is a no-op.
523+
if (from == to) {
524+
return {};
525+
}
526+
527+
// Verify the source table exists.
528+
ICEBERG_ASSIGN_OR_RAISE(auto from_exists, root_namespace_->TableExists(from));
529+
if (!from_exists) {
530+
return NoSuchTable("Source table does not exist: {}", from);
531+
}
532+
533+
// Verify the destination table does not already exist.
534+
ICEBERG_ASSIGN_OR_RAISE(auto to_exists, root_namespace_->TableExists(to));
535+
if (to_exists) {
536+
return AlreadyExists("Destination table already exists: {}", to);
537+
}
538+
539+
// Get the metadata location from the source table.
540+
ICEBERG_ASSIGN_OR_RAISE(auto metadata_location,
541+
root_namespace_->GetTableMetadataLocation(from));
542+
543+
// Register the destination with the same metadata location.
544+
ICEBERG_RETURN_UNEXPECTED(root_namespace_->RegisterTable(to, metadata_location));
545+
546+
// Unregister the source table.
547+
ICEBERG_RETURN_UNEXPECTED(root_namespace_->UnregisterTable(from));
548+
549+
return {};
522550
}
523551

524552
Result<std::shared_ptr<Table>> InMemoryCatalog::LoadTable(

src/iceberg/test/in_memory_catalog_test.cc

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,49 @@ TEST_F(InMemoryCatalogTest, DropTable) {
305305
EXPECT_THAT(result, IsOk());
306306
}
307307

308+
TEST_F(InMemoryCatalogTest, RenameTable) {
309+
// Create a table first.
310+
TableIdentifier ident{.ns = {}, .name = "t1"};
311+
auto schema = std::make_shared<Schema>(
312+
std::vector<SchemaField>{SchemaField::MakeRequired(1, "x", int64())},
313+
/*schema_id=*/1);
314+
auto spec = PartitionSpec::Unpartitioned();
315+
auto sort_order = SortOrder::Unsorted();
316+
317+
ICEBERG_UNWRAP_OR_FAIL(
318+
auto table, catalog_->CreateTable(ident, schema, spec, sort_order,
319+
GenerateTestTableLocation(ident.name), {}));
320+
321+
// Rename to itself is a no-op.
322+
EXPECT_THAT(catalog_->RenameTable(ident, ident), IsOk());
323+
324+
// Rename non-existent source table.
325+
TableIdentifier nonexist{.ns = {}, .name = "nonexist"};
326+
EXPECT_THAT(catalog_->RenameTable(nonexist, ident), IsError(ErrorKind::kNoSuchTable));
327+
328+
// Rename to an existing destination table.
329+
TableIdentifier ident2{.ns = {}, .name = "t2"};
330+
ICEBERG_UNWRAP_OR_FAIL(
331+
auto table2, catalog_->CreateTable(ident2, schema, spec, sort_order,
332+
GenerateTestTableLocation(ident2.name), {}));
333+
EXPECT_THAT(catalog_->RenameTable(ident, ident2), IsError(ErrorKind::kAlreadyExists));
334+
335+
// Drop ident2 to clear the destination, then rename.
336+
EXPECT_THAT(catalog_->DropTable(ident2, /*purge=*/false), IsOk());
337+
338+
// Rename ident -> ident2.
339+
TableIdentifier renamed{.ns = {}, .name = "t2"};
340+
EXPECT_THAT(catalog_->RenameTable(ident, renamed), IsOk());
341+
EXPECT_THAT(catalog_->TableExists(ident), HasValue(::testing::Eq(false)));
342+
EXPECT_THAT(catalog_->TableExists(renamed), HasValue(::testing::Eq(true)));
343+
344+
// Load the renamed table to verify it's intact.
345+
auto loaded = catalog_->LoadTable(renamed);
346+
ASSERT_THAT(loaded, IsOk());
347+
EXPECT_EQ(loaded.value()->name().name, "t2");
348+
EXPECT_EQ(loaded.value()->uuid(), table->uuid());
349+
}
350+
308351
TEST_F(InMemoryCatalogTest, Namespace) {
309352
Namespace ns{.levels = {"n1", "n2"}};
310353
std::unordered_map<std::string, std::string> properties = {{"prop1", "val1"},

0 commit comments

Comments
 (0)