diff --git a/src/iceberg/catalog/memory/in_memory_catalog.cc b/src/iceberg/catalog/memory/in_memory_catalog.cc index d0797e728..86d832388 100644 --- a/src/iceberg/catalog/memory/in_memory_catalog.cc +++ b/src/iceberg/catalog/memory/in_memory_catalog.cc @@ -422,6 +422,11 @@ Result> InMemoryCatalog::CreateTable( const std::string& location, const std::unordered_map& properties) { std::unique_lock lock(mutex_); + ICEBERG_ASSIGN_OR_RAISE(auto namespace_exists, + root_namespace_->NamespaceExists(identifier.ns)); + if (!namespace_exists) { + return NoSuchNamespace("Table namespace does not exist: {}", identifier.ns); + } if (root_namespace_->TableExists(identifier).value_or(false)) { return AlreadyExists("Table already exists: {}", identifier); } @@ -609,7 +614,9 @@ Result> InMemoryCatalog::RegisterTable( TableMetadataUtil::Read(*file_io_, metadata_file_location)); std::unique_lock lock(mutex_); - if (!root_namespace_->NamespaceExists(identifier.ns)) { + ICEBERG_ASSIGN_OR_RAISE(auto namespace_exists, + root_namespace_->NamespaceExists(identifier.ns)); + if (!namespace_exists) { return NoSuchNamespace("Table namespace does not exist: {}", identifier.ns); } if (!root_namespace_->RegisterTable(identifier, metadata_file_location)) { diff --git a/src/iceberg/test/in_memory_catalog_test.cc b/src/iceberg/test/in_memory_catalog_test.cc index b2c88f571..db90aa14a 100644 --- a/src/iceberg/test/in_memory_catalog_test.cc +++ b/src/iceberg/test/in_memory_catalog_test.cc @@ -158,6 +158,53 @@ TEST_F(InMemoryCatalogTest, RegisterTable) { ASSERT_EQ(table.value()->location(), "s3://bucket/test/location"); } +TEST_F(InMemoryCatalogTest, CreateTableNonexistentNamespace) { + TableIdentifier table_ident{.ns = Namespace{.levels = {"missing"}}, .name = "t1"}; + auto schema = std::make_shared( + std::vector{SchemaField::MakeRequired(1, "id", int64())}, + /*schema_id=*/1); + auto spec = PartitionSpec::Unpartitioned(); + auto sort_order = SortOrder::Unsorted(); + + // Use an explicit location whose metadata directory already exists (the local + // FileIO does not create parent dirs). A write-before-validate bug would land a + // detectable orphan there. GenerateTestTableLocation is unique per test and + // auto-cleaned via created_temp_paths_. + auto table_location = GenerateTestTableLocation(table_ident.name); + + auto table = + catalog_->CreateTable(table_ident, schema, spec, sort_order, table_location, {}); + EXPECT_THAT(table, IsError(ErrorKind::kNoSuchNamespace)); + + // The namespace check must run before any metadata file is written, so a + // failed create must leave no orphaned metadata file behind. + std::error_code ec; + size_t metadata_files = 0; + for (auto it = std::filesystem::recursive_directory_iterator(table_location, ec); + it != std::filesystem::recursive_directory_iterator(); it.increment(ec)) { + if (it->path().extension() == ".json") { + ++metadata_files; + } + } + EXPECT_EQ(metadata_files, 0); +} + +TEST_F(InMemoryCatalogTest, RegisterTableNonexistentNamespace) { + TableIdentifier table_ident{.ns = Namespace{.levels = {"missing"}}, .name = "t1"}; + + ICEBERG_UNWRAP_OR_FAIL(auto metadata, + ReadTableMetadataFromResource("TableMetadataV2Valid.json")); + auto table_location = GenerateTestTableLocation(table_ident.name); + auto metadata_location = std::format("{}v1.metadata.json", table_location); + ASSERT_THAT(TableMetadataUtil::Write(*file_io_, metadata_location, *metadata), IsOk()); + + // Registering into a nonexistent namespace reports NoSuchNamespace. The dead + // if (!NamespaceExists(...)) check tested the Result's has_value() rather + // than the contained bool, so this previously fell through to kUnknownError. + auto table = catalog_->RegisterTable(table_ident, metadata_location); + EXPECT_THAT(table, IsError(ErrorKind::kNoSuchNamespace)); +} + TEST_F(InMemoryCatalogTest, RefreshTable) { TableIdentifier table_ident{.ns = {}, .name = "t1"}; auto schema = std::make_shared(