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

refactor(query): optimize show drop tables #16370

Merged
merged 7 commits into from
Sep 4, 2024

Conversation

TCeason
Copy link
Collaborator

@TCeason TCeason commented Sep 2, 2024

I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/

Summary

  1. add get_single_table_history api
  2. optimize show drop tables
  3. get_table_history -> get_tables_history

Performance

==> Running Q2: internal/queries/02.sql
select name from system.tables where name in ('t_1', 't_2');
3
Q2[1] succeeded in 0.045 seconds
Q2[2] succeeded in 0.044 seconds
Q2[3] succeeded in 0.046 seconds

==> Running Q3: internal/queries/03.sql
select name from system.tables_with_history where name in ('t_1', 't_2');
3
Q3[1] succeeded in 0.046 seconds
Q3[2] succeeded in 0.045 seconds
Q3[3] succeeded in 0.046 seconds

Tests

  • Unit Test
  • Logic Test
  • Benchmark Test
  • No Test - Explain why

Type of change

  • Bug Fix (non-breaking change which fixes an issue)
  • New Feature (non-breaking change which adds functionality)
  • Breaking Change (fix or feature that could cause existing functionality not to work as expected)
  • Documentation Update
  • Refactoring
  • Performance Improvement
  • Other (please describe):

This change is Reviewable

@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Sep 2, 2024
@github-actions github-actions bot added the pr-refactor this PR changes the code base without new features or bugfix label Sep 2, 2024
@dosubot dosubot bot added the A-query Area: databend query label Sep 2, 2024
@TCeason TCeason force-pushed the optimzie_show_drop_tables branch from 16d470c to f9c867c Compare September 2, 2024 12:39
@TCeason TCeason added the ci-benchmark-local Benchmark: run only local test label Sep 2, 2024
@TCeason TCeason mentioned this pull request Sep 2, 2024
11 tasks
@TCeason TCeason marked this pull request as draft September 2, 2024 14:34
@TCeason TCeason marked this pull request as ready for review September 3, 2024 01:18
@TCeason TCeason force-pushed the optimzie_show_drop_tables branch from 2bd2ffc to 14c921d Compare September 3, 2024 05:45
@databendlabs databendlabs deleted a comment from github-actions bot Sep 3, 2024
@databendlabs databendlabs deleted a comment from github-actions bot Sep 3, 2024
@TCeason TCeason removed the ci-benchmark-local Benchmark: run only local test label Sep 3, 2024
Copy link
Member

@drmingdrmer drmingdrmer left a comment

Choose a reason for hiding this comment

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

Reviewed 4 of 20 files at r1, 1 of 19 files at r2, 1 of 13 files at r3, 1 of 4 files at r4, all commit messages.
Reviewable status: 1 of 20 files reviewed, 2 unresolved discussions (waiting on @dantengsky and @TCeason)


src/meta/api/src/schema_api_impl.rs line 4008 at r5 (raw file):

    now: &DateTime<Utc>,
    tb_id_list: TableIdList,
    inner_keys: &[String],

use typed key slice such as &[TableId] instead of String.

And it looks like tb_id_list and inner_keys are duplicated.


src/meta/api/src/schema_api_impl.rs line 4027 at r5 (raw file):

            }

            tb_metas.push((table_id, tb_meta_seq, tb_meta));

Use SeqV<tb_meta> to replace tb_meta_seq, tb_meta so that the caller knows which instance the seq number belongs to.

And return TableId instead. Avoid using raw u64 whenever a typed Id can be used.

@TCeason TCeason requested a review from drmingdrmer September 3, 2024 11:08
@TCeason TCeason added the ci-benchmark-local Benchmark: run only local test label Sep 3, 2024
Copy link
Contributor

github-actions bot commented Sep 3, 2024

Docker Image for PR

  • tag: pr-16370-740c846-1725368815

note: this image tag is only available for internal use,
please check the internal doc for more details.

Copy link
Contributor

github-actions bot commented Sep 3, 2024

Copy link
Member

@drmingdrmer drmingdrmer left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 20 files at r1, 4 of 19 files at r2, 10 of 13 files at r3, 1 of 2 files at r5, 4 of 4 files at r7, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dantengsky and @TCeason)

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Sep 3, 2024
Copy link
Contributor

github-actions bot commented Sep 3, 2024

Docker Image for PR

  • tag: pr-16370-a6acabb-1725380513

note: this image tag is only available for internal use,
please check the internal doc for more details.

Copy link
Contributor

github-actions bot commented Sep 3, 2024

@TCeason
Copy link
Collaborator Author

TCeason commented Sep 4, 2024

cc @BohuTANG Please review this pr.

@TCeason TCeason requested a review from BohuTANG September 4, 2024 03:33
Copy link
Member

@drmingdrmer drmingdrmer left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @BohuTANG and @dantengsky)

Copy link
Member

@drmingdrmer drmingdrmer left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @BohuTANG and @dantengsky)

@drmingdrmer drmingdrmer merged commit bfab76d into databendlabs:main Sep 4, 2024
103 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-query Area: databend query ci-benchmark-local Benchmark: run only local test lgtm This PR has been approved by a maintainer pr-refactor this PR changes the code base without new features or bugfix size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants