Move filtered SMJ Full filtered join out of join_partial phase#13369
Move filtered SMJ Full filtered join out of join_partial phase#13369alamb merged 3 commits intoapache:mainfrom
join_partial phase#13369Conversation
github-actions
bot
added
physical-expr
| Some(Box::new(col_lt_col_filter)), | ||
| ) | ||
| .run_test(&[JoinTestType::NljHj], false) | ||
| .run_test(&[NljHj, HjSmj], false) |
There was a problem hiding this comment.
works now, adding HJ vs SMJ test back
| #Alice 100 Alice 2 | ||
| #Alice 50 NULL NULL | ||
| #Bob 1 NULL NULL | ||
| query TITI rowsort |
| let mut first_row_idx = 0; | ||
| let mut seen_false = false; | ||
|
|
||
| for i in 0..row_indices_length { |
There was a problem hiding this comment.
the mask processing is more complex compared to other join types, I'm planning to add more tests and documentation preferably in follow up PR, but if its a blocker for the review I'll update this PR
|
During this work there are some opportunities found to clean up/document/improve testing for existing SMJ code. Planning to file a separate PR for it |
|
@andygrove cc |
| Some(corrected_mask.finish()) | ||
| } | ||
| JoinType::Full => { | ||
| let mut mask: Vec<Option<bool>> = vec![Some(true); row_indices_length]; |
There was a problem hiding this comment.
Could use booleanbuilder?
There was a problem hiding this comment.
unlike to other join types for this one its needed to update current array, the builder does append only afaik
|
Nice work @comphead |
|
Awesome -- thank you @comphead -- the effort you are making to get Sort merge join into shape is very cool. Thanks also to @Dandandan for the review |
…che#13369) * Move filtered SMJ Full filtered join out of `join_partial` phase * Move filtered SMJ Full filtered join out of `join_partial` phase * Move filtered SMJ Full filtered join out of `join_partial` phase
3 participants
Which issue does this PR close?
Closes #12359
Closes #10659
Rationale for this change
Move the Full Outer filtered SMJ join out of
join_partialphase to evaluate filter expressions properly and keep track of previous related expressions already evaluated for the same rowWhat changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?