feat(watermark): clean state for top n by watermark #8107
Conversation
…hao/watermark_top_n
Codecov Report
@@ Coverage Diff @@
## main #8107 +/- ##
==========================================
- Coverage 71.21% 71.19% -0.03%
==========================================
Files 1126 1126
Lines 181215 181265 +50
==========================================
- Hits 129053 129051 -2
- Misses 52162 52214 +52
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| } else { | ||
| // TODO(yuhao): handle watermark in descending case | ||
| } | ||
| Ok(Some(watermark)) |
There was a problem hiding this comment.
can we emit the watermark in descending case? it might retract the lower values from the current topN result.
There was a problem hiding this comment.
Oh not like this. Will fix.
| watermark: Watermark, | ||
| ) -> StreamExecutorResult<Option<Watermark>> { | ||
| Ok(if watermark.col_idx == self.storage_key_indices[0] { | ||
| Some(watermark) |
There was a problem hiding this comment.
ditto. descending case. I think we need to compare the last key with the watermark here.
|
This PR has been open for 60 days with no activity. Could you please update the status? Feel free to ping a reviewer if you are waiting for review. |
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
#7810
#8106
TODO:
add tests.
Checklist For Contributors
./risedev check(or alias,./risedev c)Checklist For Reviewers
Documentation
Click here for Documentation
Types of user-facing changes
Please keep the types that apply to your changes, and remove the others.
Release note