feat(batch): support hash join non-equi condition in java frontend #1713
Conversation
…pport_non-equi_condition_in_java_front_end
…pport_non-equi_condition_in_java_front_end
|
|
…pport_non-equi_condition_in_java_front_end
yuhao-su
changed the title
Codecov Report
@@ Coverage Diff @@
## main #1713 +/- ##
==========================================
+ Coverage 71.29% 71.37% +0.08%
==========================================
Files 599 599
Lines 77672 77722 +50
==========================================
+ Hits 55377 55476 +99
+ Misses 22295 22246 -49
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
legacy/planner/src/main/java/com/risingwave/planner/rel/physical/join/RwBatchHashJoin.java
Outdated
Show resolved
Hide resolved
| @@ -161,19 +161,19 @@ impl<K: HashKey + Send + Sync> Executor for HashJoinExecutor<K> { | |||
| match take(&mut self.state) { | |||
| HashJoinState::FirstProbe(probe_table) => { | |||
| let ret = self.probe(true, probe_table).await?; | |||
| if let Some(data_chunk) = ret { | |||
| if let Some(data_chunk) = ret && data_chunk.cardinality() > 0 { | |||
There was a problem hiding this comment.
If we assume that child executor should never return 0 cardinality data chunk, we should return an error rather skip it.
There was a problem hiding this comment.
It's possible for an executor to generate a 0 cardinality chunk, but it just shouldn't output it to the downstream executor. Why consider it as an error.
There was a problem hiding this comment.
I see, but why we may produce an empty data chunk?
There was a problem hiding this comment.
One possible situation is inner join with non-equi conditions. In our implementation, we first generate a chunk with equi condition and then filter out result by non-equi condition. If we are unlucky, all result rows are filtered out, the we get and 0 cardinality chunk. This may happen in other executors as well. We should investigate that later.
There was a problem hiding this comment.
I prefer to implement never return 0 len data chunk. But we can do this later after refactored with stream api.
| ensure!(probe_data_chunk.cardinality() > 0); | ||
| // TODO(yuhao): We should make sure the output chunk of upstream executor | ||
| // has cardinality > 0. | ||
| // ensure!(probe_data_chunk.cardinality() > 0); |
There was a problem hiding this comment.
Because we can't ensure this here until we check all executor implementations to make sure output chunk has cardinality > 0
What's changed and what's your intention?
Checklist
Refer to a related PR or issue link (optional)
#1621