Skip to content

Conversation

@BugenZhao
Copy link
Member

@BugenZhao BugenZhao commented Mar 14, 2022

What's changed and what's your intention?

This PR introduces delete executor in batch crate.

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)

A step of #16.

Signed-off-by: Bugen Zhao <[email protected]>
Signed-off-by: Bugen Zhao <[email protected]>
Signed-off-by: Bugen Zhao <[email protected]>
Signed-off-by: Bugen Zhao <[email protected]>
Signed-off-by: Bugen Zhao <[email protected]>
@github-actions github-actions bot added the type/feature Type: New feature. label Mar 14, 2022
@codecov
Copy link

codecov bot commented Mar 14, 2022

Codecov Report

Merging #883 (7d1c536) into main (d6d3d81) will increase coverage by 0.02%.
The diff coverage is 79.11%.

@@             Coverage Diff              @@
##               main     #883      +/-   ##
============================================
+ Coverage     72.30%   72.32%   +0.02%     
  Complexity     2766     2766              
============================================
  Files           934      935       +1     
  Lines         54867    54992     +125     
  Branches       1787     1787              
============================================
+ Hits          39670    39772     +102     
- Misses        14307    14330      +23     
  Partials        890      890              
Flag Coverage Δ
java 61.22% <ø> (ø)
rust 76.78% <79.11%> (+0.01%) ⬆️

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

Impacted Files Coverage Δ
rust/batch/src/executor/mod.rs 22.91% <0.00%> (-0.49%) ⬇️
rust/common/src/catalog/mod.rs 66.66% <ø> (ø)
rust/stream/src/executor/test_utils.rs 75.00% <ø> (+1.38%) ⬆️
rust/common/src/catalog/schema.rs 80.95% <50.00%> (-19.05%) ⬇️
rust/batch/src/executor/insert.rs 83.65% <66.66%> (-1.11%) ⬇️
rust/batch/src/executor/delete.rs 79.06% <79.06%> (ø)
rust/compute/tests/table_v2_materialize.rs 94.52% <92.10%> (+2.62%) ⬆️
rust/source/src/table_v2.rs 94.82% <100.00%> (ø)
rust/stream/src/executor/local_simple_agg.rs 85.07% <100.00%> (ø)
rust/stream/src/executor/mview/state.rs 96.29% <100.00%> (ø)
... and 3 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

Signed-off-by: Bugen Zhao <[email protected]>
@BugenZhao BugenZhao marked this pull request as ready for review March 14, 2022 06:08
}

async fn next(&mut self) -> Result<Option<DataChunk>> {
if self.executed {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not specific to this PR, just try to confirm whether it is the standard in our system now that next on an already finished executor should be a no-op instead of err?

Copy link
Member Author

Choose a reason for hiding this comment

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

+1. Also curious about this.

Copy link
Contributor

@lmatz lmatz 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 7be7027 into main Mar 14, 2022
@BugenZhao BugenZhao deleted the bz/delete-part-1 branch March 14, 2022 07:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/feature Type: New feature.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants