Skip to content

Conversation

@lmatz
Copy link
Contributor

@lmatz lmatz commented Feb 7, 2022

What's changed and what's your intention?

As #121 describes, aggregation with distinct cannot be processed in the two-phase fashion.

Refer to a related PR or issue link (optional)

closes #121

@lmatz lmatz force-pushed the lz/shuffle_distinct branch from 67dc07f to 1fc29ee Compare February 7, 2022 12:15
@github-actions github-actions bot added the type/fix Type: Bug fix. Only for pull requests. label Feb 7, 2022
@codecov
Copy link

codecov bot commented Feb 7, 2022

Codecov Report

Merging #125 (1fc29ee) into main (78ebb70) will decrease coverage by 0.08%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #125      +/-   ##
============================================
- Coverage     74.54%   74.46%   -0.09%     
- Complexity     2645     2653       +8     
============================================
  Files           832      833       +1     
  Lines         47823    47824       +1     
  Branches       1560     1562       +2     
============================================
- Hits          35651    35611      -40     
- Misses        11374    11414      +40     
- Partials        798      799       +1     
Flag Coverage Δ
java 62.07% <100.00%> (+0.16%) ⬆️
rust 79.81% <ø> (-0.19%) ⬇️

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

Impacted Files Coverage Δ
...singwave/planner/rules/physical/BatchRuleSets.java 100.00% <ø> (ø)
...planner/rules/distributed/agg/TwoPhaseAggRule.java 100.00% <100.00%> (ø)
rust/meta/src/stream/meta.rs 58.57% <0.00%> (-37.93%) ⬇️
rust/meta/src/manager/config.rs 50.68% <0.00%> (-10.96%) ⬇️
rust/meta/src/barrier/command.rs 44.00% <0.00%> (-5.02%) ⬇️
rust/meta/src/barrier/mod.rs 65.90% <0.00%> (-4.33%) ⬇️
rust/meta/src/stream/stream_manager.rs 89.49% <0.00%> (-0.99%) ⬇️
rust/meta/src/storage/metastore.rs 95.13% <0.00%> (-0.70%) ⬇️
rust/meta/src/hummock/compaction.rs 73.85% <0.00%> (-0.66%) ⬇️
rust/meta/src/model/mod.rs 80.00% <0.00%> (ø)
... and 12 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 78ebb70...1fc29ee. Read the comment docs.

Copy link
Collaborator

@fuyufjh fuyufjh left a comment

Choose a reason for hiding this comment

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

LGTM

@lmatz lmatz merged commit 3b1ec24 into main Feb 7, 2022
@lmatz lmatz deleted the lz/shuffle_distinct branch February 7, 2022 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Aggregation function with distinct cannot use two phase aggregation

3 participants