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(proto): simplify ColumnOrder #3637

Merged
merged 6 commits into from
Jul 4, 2022
Merged

refactor(proto): simplify ColumnOrder #3637

merged 6 commits into from
Jul 4, 2022

Conversation

xxchan
Copy link
Member

@xxchan xxchan commented Jul 4, 2022

I hereby agree to the terms of the Singularity Data, Inc. Contributor License Agreement.

What's changed and what's your intention?

Trying to dedup OrderedColumnDesc, and use FieldOrder instead (#3638). But found it difficult. Simplify ColumnOrder proto first

@codecov
Copy link

codecov bot commented Jul 4, 2022

Codecov Report

Merging #3637 (2bc9945) into main (fe8142a) will increase coverage by 0.01%.
The diff coverage is 21.42%.

@@            Coverage Diff             @@
##             main    #3637      +/-   ##
==========================================
+ Coverage   74.37%   74.39%   +0.01%     
==========================================
  Files         776      776              
  Lines      110163   110134      -29     
==========================================
- Hits        81938    81930       -8     
+ Misses      28225    28204      -21     
Flag Coverage Δ
rust 74.39% <21.42%> (+0.01%) ⬆️

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

Impacted Files Coverage Δ
src/common/src/catalog/physical_table.rs 61.53% <0.00%> (+6.36%) ⬆️
src/common/src/util/sort_util.rs 81.19% <0.00%> (+0.68%) ⬆️
...rc/frontend/src/optimizer/plan_node/stream_topn.rs 54.71% <0.00%> (+5.56%) ⬆️
src/frontend/src/optimizer/property/order.rs 81.64% <0.00%> (+4.40%) ⬆️
...ontend/src/stream_fragmenter/rewrite/delta_join.rs 0.00% <0.00%> (ø)
...tend/src/optimizer/plan_node/stream_materialize.rs 91.05% <100.00%> (-0.14%) ⬇️
src/meta/src/stream/test_fragmenter.rs 99.37% <100.00%> (-0.01%) ⬇️
src/meta/src/manager/id.rs 94.94% <0.00%> (-0.57%) ⬇️
src/frontend/src/expr/utils.rs 98.99% <0.00%> (-0.26%) ⬇️
src/storage/src/hummock/iterator/merge_inner.rs 90.00% <0.00%> (+0.62%) ⬆️

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

@xxchan xxchan added the mergify/can-merge Indicates that the PR can be added to the merge queue label Jul 4, 2022
@xxchan xxchan requested a review from TennyZhuang July 4, 2022 14:36
@mergify
Copy link
Contributor

mergify bot commented Jul 4, 2022

Hey @xxchan, this pull request failed to merge and has been dequeued from the merge train. If you believe your PR failed in the merge train because of a flaky test, requeue it by commenting with @mergifyio requeue. More details can be found on the Queue: Embarked in merge train check-run.

@xxchan
Copy link
Member Author

xxchan commented Jul 4, 2022

@Mergifyio requeue

@mergify
Copy link
Contributor

mergify bot commented Jul 4, 2022

requeue

✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically

@mergify mergify bot merged commit 23a7fae into main Jul 4, 2022
@mergify mergify bot deleted the xxchan/mv-proto branch July 4, 2022 15:45
nasnoisaac pushed a commit to nasnoisaac/risingwave that referenced this pull request Aug 9, 2022
* remove return_type from ColumnOrder

* inputref->index in ColumnOrder

* lint

* apply plan

* rerun

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mergify/can-merge Indicates that the PR can be added to the merge queue type/refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants