feat(stream): temporal join support pk prefix join condition #10627
Conversation
Codecov Report
@@ Coverage Diff @@
## main #10627 +/- ##
==========================================
- Coverage 70.23% 70.22% -0.02%
==========================================
Files 1285 1285
Lines 219704 219836 +132
==========================================
+ Hits 154311 154371 +60
- Misses 65393 65465 +72
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| @@ -271,40 +386,42 @@ impl<S: StateStore, const T: JoinTypePrimitive> TemporalJoinExecutor<S, T> { | |||
| .set(self.right_table.cache.len() as i64); | |||
| match msg? { | |||
| InternalMessage::Chunk(chunk) => { | |||
| // Compact chunk, otherwise the following keys and chunk rows might fail to zip. | |||
| let chunk = chunk.compact(); | |||
There was a problem hiding this comment.
Why? chunk.rows() should only returns visible rows
There was a problem hiding this comment.
let keys = K::build(&self.left_join_keys, chunk.data_chunk())?;
for ((op, left_row), key) in chunk.rows().zip_eq_debug(keys.into_iter()) {We need to align keys with rows, but keys can't handle visible ops. Previously I also thought like you, but encounter this issue. Stream hash joins work around by compact as well.
There was a problem hiding this comment.
I guess we can filter out the empty keys for now (but the assumption is really easy to break, we need further encapsulation for this).
There was a problem hiding this comment.
Yes, this pattern is quite common(HashJoin, TopN). We can do a refactor for this pattern later.
chenzl25
deleted the
dylan/temporal_join_support_pk_prefix_join_condition
branch
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
Checklist
./risedev check(or alias,./risedev c)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