Skip to content

fix(batch): COUNT(*) from mview table #182

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Feb 9, 2022
Merged

fix(batch): COUNT(*) from mview table #182

merged 3 commits into from
Feb 9, 2022

Conversation

@BugenZhao
Copy link
Member

@BugenZhao BugenZhao commented Feb 9, 2022 •
edited
Loading

What's changed and what's your intention?

Previously MViewTable::get_data_by_columns yields None once column_ids is empty. This PR makes it generate a dummy chunk with correct cardinality instead, which behaves like BummockTable.

Checklist

  • I have written necessary docs and comments
  • I have added necessary unit tests and integration tests

Refer to a related PR or issue link (optional)

Fix #181.

@BugenZhao BugenZhao requested review from MrCroxx, lmatz and skyzh February 9, 2022 14:58
@github-actions github-actions bot added the type/fix Type: Bug fix. Only for pull requests. label Feb 9, 2022
@BugenZhao BugenZhao enabled auto-merge (squash) February 9, 2022 14:59
@codecov
Copy link

codecov bot commented Feb 9, 2022

Codecov Report

Merging #182 (d0c7d80) into main (48598e8) will increase coverage by 0.10%.
The diff coverage is 97.36%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #182      +/-   ##
============================================
+ Coverage     74.47%   74.58%   +0.10%     
  Complexity     2654     2654              
============================================
  Files           838      838              
  Lines         47891    47793      -98     
  Branches       1562     1562              
============================================
- Hits          35667    35646      -21     
+ Misses        11425    11348      -77     
  Partials        799      799
Flag Coverage Δ
java 62.08% <ø> (ø)
rust 79.98% <97.36%> (+0.17%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ust/stream/src/executor/mview/table_state_tests.rs 99.63% <96.77%> (-0.37%) ⬇️
rust/storage/src/table/mod.rs 80.55% <100.00%> (+27.43%) ⬆️
rust/meta/src/manager/config.rs 92.85% <0.00%> (-7.15%) ⬇️
rust/storage/src/hummock/cloud.rs 89.65% <0.00%> (-0.67%) ⬇️
rust/meta/src/hummock/compaction.rs 73.85% <0.00%> (-0.66%) ⬇️
rust/common/src/types/ordered_float.rs 25.49% <0.00%> (-0.34%) ⬇️
rust/meta/src/hummock/hummock_manager_tests.rs 96.15% <0.00%> (-0.20%) ⬇️
rust/meta/src/manager/id.rs 94.49% <0.00%> (-0.10%) ⬇️
rust/meta/src/rpc/service/hummock_service.rs 1.58% <0.00%> (+0.45%) ⬆️
rust/storage/src/hummock/error.rs 23.52% <0.00%> (+2.47%) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 48598e8...d0c7d80. Read the comment docs.

MrCroxx
MrCroxx approved these changes Feb 9, 2022
View reviewed changes
Copy link
Contributor

@MrCroxx MrCroxx left a comment

Choose a reason for hiding this comment

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

LGTM

@BugenZhao BugenZhao merged commit 4fa67cf into main Feb 9, 2022
@BugenZhao BugenZhao deleted the bz/fix-batch-count-star-from-mv branch February 9, 2022 15:53
@BugenZhao BugenZhao mentioned this pull request Feb 10, 2022
Merged
2 tasks
@BugenZhao BugenZhao mentioned this pull request May 14, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MrCroxx MrCroxx MrCroxx approved these changes

@skyzh skyzh Awaiting requested review from skyzh

@lmatz lmatz Awaiting requested review from lmatz

Assignees

No one assigned

Labels

type/fix Type: Bug fix. Only for pull requests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

batch: COUNT(*) from mview table gives result "0"

3 participants

@BugenZhao @MrCroxx