Skip to content
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

feat: introduce TableOperator to encasulate operation of tables #808

Merged
merged 11 commits into from
Apr 7, 2023

Conversation

Rachelint
Copy link
Contributor

@Rachelint Rachelint commented Apr 5, 2023

Which issue does this PR close?

Closes #

Rationale for this change

Now, we directly call the method in Schema for table operations, that led to tons of dulicated codes(e.g. create and drop in meta event service and interpreter, open in meta event servie and local recovery).

In this pr, TableOperator is introduced to escasulate such operations for eliminating the duplicateds.

What changes are included in this PR?

  • Introduce the TableOperator.
  • Refactor open table and close table
    • Remove them from Schema, and add register_table and unregister_table.
    • Directly call table_engine for opening and closing.
    • Combine them in TableOperator.
  • Do adapt work in related modules.

Are there any user-facing changes?

None.

How does this change test

Test exist tests.

@codecov-commenter
Copy link

codecov-commenter commented Apr 6, 2023

Codecov Report

Merging #808 (caa3c19) into main (d9b7d15) will decrease coverage by 0.14%.
The diff coverage is 19.48%.

❗ Current head caa3c19 differs from pull request most recent head 8fed4a4. Consider uploading reports for the commit 8fed4a4 to get more accurate results

@@            Coverage Diff             @@
##             main     #808      +/-   ##
==========================================
- Coverage   67.98%   67.84%   -0.14%     
==========================================
  Files         299      300       +1     
  Lines       47174    47324     +150     
==========================================
+ Hits        32073    32109      +36     
- Misses      15101    15215     +114     
Impacted Files Coverage Δ
catalog_impls/src/system_tables.rs 0.00% <0.00%> (ø)
catalog_impls/src/volatile.rs 0.00% <0.00%> (ø)
interpreters/src/table_manipulator/mod.rs 0.00% <ø> (ø)
server/src/grpc/meta_event_service/error.rs 0.00% <ø> (ø)
server/src/grpc/meta_event_service/mod.rs 0.00% <0.00%> (ø)
server/src/local_tables.rs 0.00% <0.00%> (ø)
src/setup.rs 0.00% <0.00%> (ø)
table_engine/src/engine.rs 53.52% <0.00%> (-1.56%) ⬇️
catalog_impls/src/table_based.rs 85.97% <16.66%> (+2.61%) ⬆️
catalog/src/table_operator.rs 21.34% <21.34%> (ø)
... and 4 more

... and 2 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@Rachelint Rachelint mentioned this pull request Apr 6, 2023
12 tasks
Copy link
Member

@ShiKaiWi ShiKaiWi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Rachelint Rachelint added this pull request to the merge queue Apr 7, 2023
Merged via the queue into apache:main with commit f27792c Apr 7, 2023
chunshao90 pushed a commit to chunshao90/ceresdb that referenced this pull request May 15, 2023
…ache#808)

* add `register_table` in `Schema`, adapt this change in volatile based schema impl.

* refactor table opertations.

* abort the modification in meta event service.

* add table operator to meta_event_service.

* add table operator to local recovery.

* make clippy happy.

* update integration tests.

* remove useless table operation methods, and schema id in related requests.

* address CR.
@Rachelint Rachelint deleted the split-table-opening-from-schema branch May 27, 2023 12:18
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.

3 participants