HashJoin should return Err when the right side input stream produce Err, add more join UTs to cover different join types #4373
Conversation
…rr, add more join UTs to cover different join types
| @@ -1554,7 +1554,8 @@ impl HashJoinStream { | |||
| } | |||
| Some(result.map(|x| x.0)) | |||
| } | |||
| other => { | |||
| Some(err) => Some(err), | |||
There was a problem hiding this comment.
👍 this is also found in the refactor pr https://github.com/apache/arrow-datafusion/pull/4377/files#
| // TODO should not use cross join when the join_filter exists | ||
| // https://github.com/apache/arrow-datafusion/issues/4363 |
There was a problem hiding this comment.
Perhaps our current crossjoin implementation is a little strange.
Some databases implement Crossjoin via Nest Loop Join. (cross join is logical, NLJ is physical)
There was a problem hiding this comment.
Looks like a great improvement to me (the code looks good and the tests are 💯 ) Thank you @mingmwang
|
Benchmark runs are scheduled for baseline = 3b7f767 and contender = 27ae14a. 27ae14a is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
Closes #4362.
Rationale for this change
What changes are included in this PR?
The PR also added some join UTs to cover different join types(Semi/Anti/Cross). It discloses several bugs and need to be addressed further.
Are these changes tested?
Are there any user-facing changes?