Skip to content

Commit 580b53a

Browse files
lxy-9602lxy-9602
authored
fix: LoadTableSchema returns NotExist error instead of null when table does not exist (alibaba#40)
1 parent 4661997 commit 580b53a

File tree

4 files changed

+37
-48
lines changed

4 files changed

+37
-48
lines changed

‎include/paimon/catalog/catalog.h‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -100,10 +100,8 @@ class PAIMON_EXPORT Catalog {
100100
/// @note System tables will not be supported.
101101
///
102102
/// @param identifier The identifier (database and table name) of the table to load.
103-
/// @return A result containing table schema if the table exists, or std::nullopt if it
104-
/// doesn't, or an error status on failure.
105-
virtual Result<std::optional<std::shared_ptr<Schema>>> LoadTableSchema(
106-
const Identifier& identifier) const = 0;
103+
/// @return A result containing table schema if the table exists, or an error status on failure.
104+
virtual Result<std::shared_ptr<Schema>> LoadTableSchema(const Identifier& identifier) const = 0;
107105
};
108106

109107
} // namespace paimon

‎src/paimon/core/catalog/file_system_catalog.cpp‎

Lines changed: 13 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,6 @@ class Schema;
3939
struct ArrowSchema;
4040

4141
namespace paimon {
42-
class TableSchema;
43-
4442
FileSystemCatalog::FileSystemCatalog(const std::shared_ptr<FileSystem>& fs,
4543
const std::string& warehouse)
4644
: fs_(fs), warehouse_(warehouse), logger_(Logger::GetLogger("FileSystemCatalog")) {}
@@ -104,7 +102,9 @@ Status FileSystemCatalog::CreateTable(const Identifier& identifier, ArrowSchema*
104102
return Status::Invalid(
105103
fmt::format("database {} is not exist", identifier.GetDatabaseName()));
106104
}
107-
PAIMON_ASSIGN_OR_RAISE(bool table_exist, TableExists(identifier));
105+
PAIMON_ASSIGN_OR_RAISE(std::optional<std::shared_ptr<TableSchema>> latest_schema,
106+
TableSchemaExists(identifier));
107+
bool table_exist = (latest_schema != std::nullopt);
108108
if (table_exist) {
109109
if (ignore_if_exists) {
110110
return Status::OK();
@@ -128,17 +128,14 @@ Status FileSystemCatalog::CreateTable(const Identifier& identifier, ArrowSchema*
128128
return Status::OK();
129129
}
130130

131-
Result<bool> FileSystemCatalog::TableExists(const Identifier& identifier) const {
131+
Result<std::optional<std::shared_ptr<TableSchema>>> FileSystemCatalog::TableSchemaExists(
132+
const Identifier& identifier) const {
132133
if (IsSystemTable(identifier)) {
133-
return Status::NotImplemented("do not support checking TableExists for system table.");
134+
return Status::NotImplemented(
135+
"do not support checking TableSchemaExists for system table.");
134136
}
135137
SchemaManager schema_manager(fs_, NewDataTablePath(warehouse_, identifier));
136-
PAIMON_ASSIGN_OR_RAISE(std::optional<std::shared_ptr<TableSchema>> latest_schema,
137-
schema_manager.Latest());
138-
if (latest_schema == std::nullopt) {
139-
return false;
140-
}
141-
return true;
138+
return schema_manager.Latest();
142139
}
143140

144141
bool FileSystemCatalog::IsSystemDatabase(const std::string& db_name) {
@@ -212,19 +209,14 @@ Result<bool> FileSystemCatalog::TableExistsInFileSystem(const std::string& table
212209
}
213210
}
214211

215-
Result<std::optional<std::shared_ptr<Schema>>> FileSystemCatalog::LoadTableSchema(
212+
Result<std::shared_ptr<Schema>> FileSystemCatalog::LoadTableSchema(
216213
const Identifier& identifier) const {
217-
if (IsSystemTable(identifier)) {
218-
return Status::NotImplemented("do not support loading schema for system table.");
219-
}
220-
SchemaManager schema_manager(fs_, NewDataTablePath(warehouse_, identifier));
221214
PAIMON_ASSIGN_OR_RAISE(std::optional<std::shared_ptr<TableSchema>> latest_schema,
222-
schema_manager.Latest());
223-
if (latest_schema.has_value()) {
224-
std::shared_ptr<Schema> schema = std::make_shared<SchemaImpl>(*latest_schema);
225-
return std::optional<std::shared_ptr<Schema>>(schema);
215+
TableSchemaExists(identifier));
216+
if (!latest_schema) {
217+
return Status::NotExist(fmt::format("{} not exist", identifier.ToString()));
226218
}
227-
return std::optional<std::shared_ptr<Schema>>();
219+
return std::make_shared<SchemaImpl>(*latest_schema);
228220
}
229221

230222
} // namespace paimon

‎src/paimon/core/catalog/file_system_catalog.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
struct ArrowSchema;
3030

3131
namespace paimon {
32-
32+
class TableSchema;
3333
class FileSystem;
3434
class Identifier;
3535
class Logger;
@@ -49,8 +49,7 @@ class FileSystemCatalog : public Catalog {
4949

5050
Result<std::vector<std::string>> ListDatabases() const override;
5151
Result<std::vector<std::string>> ListTables(const std::string& database_names) const override;
52-
Result<std::optional<std::shared_ptr<Schema>>> LoadTableSchema(
53-
const Identifier& identifier) const override;
52+
Result<std::shared_ptr<Schema>> LoadTableSchema(const Identifier& identifier) const override;
5453

5554
private:
5655
static std::string NewDatabasePath(const std::string& warehouse, const std::string& db_name);
@@ -59,7 +58,8 @@ class FileSystemCatalog : public Catalog {
5958
static bool IsSpecifiedSystemTable(const Identifier& identifier);
6059
static bool IsSystemTable(const Identifier& identifier);
6160
Result<bool> DataBaseExists(const std::string& db_name) const;
62-
Result<bool> TableExists(const Identifier& identifier) const;
61+
Result<std::optional<std::shared_ptr<TableSchema>>> TableSchemaExists(
62+
const Identifier& identifier) const;
6363

6464
Status CreateDatabaseImpl(const std::string& db_name,
6565
const std::map<std::string, std::string>& options);

‎src/paimon/core/catalog/file_system_catalog_test.cpp‎

Lines changed: 18 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -180,10 +180,9 @@ TEST(FileSystemCatalogTest, TestCreateTableWithBlob) {
180180
ASSERT_OK_AND_ASSIGN(std::vector<std::string> table_names, catalog.ListTables("db1"));
181181
ASSERT_EQ(1, table_names.size());
182182
ASSERT_EQ(table_names[0], "tbl1");
183-
ASSERT_OK_AND_ASSIGN(std::optional<std::shared_ptr<Schema>> table_schema,
183+
ASSERT_OK_AND_ASSIGN(std::shared_ptr<Schema> table_schema,
184184
catalog.LoadTableSchema(Identifier("db1", "tbl1")));
185-
ASSERT_TRUE(table_schema.has_value());
186-
ASSERT_OK_AND_ASSIGN(auto arrow_schema, (*table_schema)->GetArrowSchema());
185+
ASSERT_OK_AND_ASSIGN(auto arrow_schema, table_schema->GetArrowSchema());
187186
auto loaded_schema = arrow::ImportSchema(arrow_schema.get()).ValueOrDie();
188187
ASSERT_TRUE(typed_schema.Equals(loaded_schema));
189188
ArrowSchemaRelease(&schema);
@@ -336,32 +335,32 @@ TEST(FileSystemCatalogTest, TestValidateTableSchema) {
336335
ASSERT_OK(catalog.CreateTable(Identifier("db1", "tbl1"), &schema, {"f1"}, {}, options,
337336
/*ignore_if_exists=*/false));
338337

339-
ASSERT_OK_AND_ASSIGN(std::optional<std::shared_ptr<Schema>> table_schema,
340-
catalog.LoadTableSchema(Identifier("db0", "tbl0")));
341-
ASSERT_FALSE(table_schema.has_value());
342-
ASSERT_OK_AND_ASSIGN(table_schema, catalog.LoadTableSchema(Identifier("db1", "tbl1")));
343-
ASSERT_TRUE(table_schema.has_value());
344-
ASSERT_EQ(0, (*table_schema)->Id());
345-
ASSERT_EQ(3, (*table_schema)->HighestFieldId());
346-
ASSERT_EQ(1, (*table_schema)->PartitionKeys().size());
347-
ASSERT_EQ(0, (*table_schema)->PrimaryKeys().size());
348-
ASSERT_EQ(-1, (*table_schema)->NumBuckets());
349-
ASSERT_FALSE((*table_schema)->Comment().has_value());
350-
std::vector<std::string> field_names = (*table_schema)->FieldNames();
338+
ASSERT_NOK_WITH_MSG(catalog.LoadTableSchema(Identifier("db0", "tbl0")),
339+
"Identifier{database=\'db0\', table=\'tbl0\'} not exist");
340+
ASSERT_OK_AND_ASSIGN(std::shared_ptr<Schema> table_schema,
341+
catalog.LoadTableSchema(Identifier("db1", "tbl1")));
342+
ASSERT_EQ(0, table_schema->Id());
343+
ASSERT_EQ(3, table_schema->HighestFieldId());
344+
ASSERT_EQ(1, table_schema->PartitionKeys().size());
345+
ASSERT_EQ(0, table_schema->PrimaryKeys().size());
346+
ASSERT_EQ(-1, table_schema->NumBuckets());
347+
ASSERT_FALSE(table_schema->Comment().has_value());
348+
std::vector<std::string> field_names = table_schema->FieldNames();
351349
std::vector<std::string> expected_field_names = {"f0", "f1", "f2", "f3"};
352350
ASSERT_EQ(field_names, expected_field_names);
353351

354-
ASSERT_OK_AND_ASSIGN(auto arrow_schema, (*table_schema)->GetArrowSchema());
352+
ASSERT_OK_AND_ASSIGN(auto arrow_schema, table_schema->GetArrowSchema());
355353
auto loaded_schema = arrow::ImportSchema(arrow_schema.get()).ValueOrDie();
356354
ASSERT_TRUE(typed_schema.Equals(loaded_schema));
357355

358356
ASSERT_OK_AND_ASSIGN(auto fs, FileSystemFactory::Get("local", dir->Str(), {}));
359357
ASSERT_OK(fs->Delete(PathUtil::JoinPath(dir->Str(), "db1.db/tbl1/schema/schema-0")));
360-
ASSERT_OK_AND_ASSIGN(table_schema, catalog.LoadTableSchema(Identifier("db1", "tbl1")));
361-
ASSERT_FALSE(table_schema.has_value());
358+
359+
ASSERT_NOK_WITH_MSG(catalog.LoadTableSchema(Identifier("db1", "tbl1")),
360+
"Identifier{database=\'db1\', table=\'tbl1\'} not exist");
362361

363362
ASSERT_NOK_WITH_MSG(catalog.LoadTableSchema(Identifier("db1", "tbl$11")),
364-
"do not support loading schema for system table.");
363+
"do not support checking TableSchemaExists for system table.");
365364
ArrowSchemaRelease(&schema);
366365
}
367366

0 commit comments

Comments
 (0)