fix bug: right semi join can't support the filter #4327
Conversation
| let stream = join.execute(0, task_ctx)?; | ||
| let batches = common::collect(stream).await?; | ||
|
|
||
| let expected = vec![ |
There was a problem hiding this comment.
If don't fix this bug, the result will be
[
"+----+----+----+",
"| a1 | b1 | c1 |",
"+----+----+----+",
"+----+----+----+",
]
liukun4515
force-pushed
the
hash_join_right_semi_#4247
branch
from
64fa5dc to
3f95e42
Compare
liukun4515
force-pushed
the
hash_join_right_semi_#4247
branch
from
3f95e42 to
d884bb5
Compare
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn join_left_semi_with_filter() -> Result<()> { |
There was a problem hiding this comment.
add test cases to ensure left semi with filter is correct.
|
This bug is caused by this commit d6f0b12#r90728800 |
| &keys_values, | ||
| *null_equals_null, | ||
| )? { | ||
| left_indices.append(i); |
There was a problem hiding this comment.
Ah good find. I missed the case where this would be used.
Thanks for adding the unit test!
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
Thanks @liukun4515 and @Dandandan
| Column::new_with_schema("b1", &right.schema())?, | ||
| )]; | ||
|
|
||
| // build filter right.b2 > 4 |
There was a problem hiding this comment.
Ah, the key is this filter. 👍
There was a problem hiding this comment.
Ah, the key is this filter. 👍
Yes, we can add filter to filter left row or right row, and get the new result.
|
Benchmark runs are scheduled for baseline = 94cd982 and contender = 92325bf. 92325bf is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
After I go through the code of |
|
find a new bug for the for example with sql In the current code, we will get the empty result because of break in the |
|
Yes, it is a bug indeed. |
5 participants
Which issue does this PR close?
part of #4247
right semijoin with filter and add test case for this bugleft semijoin with filterTODO in the next PR: fix the
right antijoin with filter; Meet some issue about the codebase, will file a new PR to fix that.@alamb @Dandandan
Rationale for this change
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?