Skip to content

feat: RegisterTable support for InMemoryCatalog#142

Merged
Xuanwo merged 2 commits intoapache:mainfrom
lishuxu:feature/register-table
Jul 21, 2025
Merged

feat: RegisterTable support for InMemoryCatalog#142
Xuanwo merged 2 commits intoapache:mainfrom
lishuxu:feature/register-table

Conversation

@lishuxu
Copy link
Copy Markdown
Contributor

@lishuxu lishuxu commented Jul 7, 2025

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.

auto status = TableMetadataUtil::Write(*file_io_, temp_filepath_, *metadata);
EXPECT_THAT(status, IsOk());

auto table = catalog_->RegisterTable(tableIdent, temp_filepath_);
Copy link
Copy Markdown
Member

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.

Copy link
Copy Markdown
Contributor Author

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

@lishuxu lishuxu force-pushed the feature/register-table branch from aa35ed0 to fa57abd Compare July 15, 2025 04:13
@lishuxu lishuxu force-pushed the feature/register-table branch from 437168d to bb1cada Compare July 17, 2025 02:45
@lishuxu lishuxu force-pushed the feature/register-table branch from bb1cada to 4d944db Compare July 17, 2025 03:05
Copy link
Copy Markdown
Member

@Xuanwo Xuanwo left a 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!

@Xuanwo Xuanwo merged commit 0279fdc into apache:main Jul 21, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants