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

fix(frontend): occasional table v2 create error #267

Merged
merged 4 commits into from
Feb 11, 2022

Conversation

BugenZhao
Copy link
Member

What's changed and what's your intention?

This PR ensures that the base table is created before creating the associated materialized view.

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)

Close #230.

@codecov
Copy link

codecov bot commented Feb 11, 2022

Codecov Report

Merging #267 (816ccbe) into main (f69f5f6) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #267      +/-   ##
============================================
- Coverage     74.07%   74.06%   -0.01%     
  Complexity     2665     2665              
============================================
  Files           850      850              
  Lines         48119    48124       +5     
  Branches       1583     1584       +1     
============================================
  Hits          35643    35643              
- Misses        11666    11671       +5     
  Partials        810      810              
Flag Coverage Δ
java 61.94% <0.00%> (-0.03%) ⬇️
rust 79.31% <0.00%> (ø)

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

Impacted Files Coverage Δ
...singwave/execution/handler/CreateTableHandler.java 0.00% <0.00%> (ø)
rust/storage/src/table/simple_manager.rs 83.58% <0.00%> (ø)

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 f69f5f6...816ccbe. Read the comment docs.

Copy link
Contributor

@skyzh skyzh left a comment

Choose a reason for hiding this comment

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

I would still recommend returning an Error instead of panic. If we panic, the thread would hang, and the e2e test will never end (until reaching timeout)

@BugenZhao
Copy link
Member Author

I would still recommend returning an Error instead of panic. If we panic, the thread would hang, and the e2e test will never end (until reaching timeout)

Okay. Let's make it temporarily an error to see whether this PR really fix this problem :). Considering the inexistence of associated base table is an unrecoverable error, it should be a panic in the future.

@skyzh skyzh enabled auto-merge (squash) February 11, 2022 10:23
@BugenZhao BugenZhao enabled auto-merge (squash) February 11, 2022 10:27
@BugenZhao BugenZhao merged commit 668cc37 into main Feb 11, 2022
@BugenZhao BugenZhao deleted the bz/fix-occasional-table-v2-create-error branch February 11, 2022 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/fix Bug fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

table_v2: occasional creating error "base table not exists"
4 participants