-
Notifications
You must be signed in to change notification settings - Fork 71
feat: RegisterTable support for InMemoryCatalog #142
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
test/in_memory_catalog_test.cc
Outdated
| auto status = TableMetadataUtil::Write(*file_io_, temp_filepath_, *metadata); | ||
| EXPECT_THAT(status, IsOk()); | ||
|
|
||
| auto table = catalog_->RegisterTable(tableIdent, temp_filepath_); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not directly use std::filesystem::path path{GetResourcePath("TableMetadataV2Valid.json")} to locate the file? We don't need to read and write the metadata file again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The table metadata organization requires a separate xx.metadata.json file to store the table metadata
aa35ed0 to
fa57abd
Compare
437168d to
bb1cada
Compare
bb1cada to
4d944db
Compare
Xuanwo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @lishuxu for working on this!
Note: Since the LoadTable interface needs to return a Table object that holds a shared_from_this pointer to the catalog, I remove InMemoryCatalog inheritance from Catalog and instead directly implement the interface in InMemoryCatalog.