-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
infer right side nullability for LEFT join #5748
Changes from all commits
d135f69
1674be4
3302426
b34e1b5
bf8ed7b
b8af34d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1041,20 +1041,44 @@ pub fn build_join_schema( | |
right: &DFSchema, | ||
join_type: &JoinType, | ||
) -> Result<DFSchema> { | ||
let right_fields = right.fields(); | ||
let left_fields = left.fields(); | ||
|
||
let fields: Vec<DFField> = match join_type { | ||
JoinType::Inner | JoinType::Left | JoinType::Full | JoinType::Right => { | ||
let right_fields = right.fields().iter(); | ||
let left_fields = left.fields().iter(); | ||
JoinType::Inner | JoinType::Full | JoinType::Right => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right should be treated symmetrically opposite of Left. Full should be nullified on both sides. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
// left then right | ||
left_fields.chain(right_fields).cloned().collect() | ||
left_fields | ||
.iter() | ||
.chain(right_fields.iter()) | ||
.cloned() | ||
.collect() | ||
} | ||
JoinType::Left => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll fix other types of joins on separate PR |
||
// left then right, right set to nullable in case of not matched scenario | ||
let right_fields_nullable: Vec<DFField> = right_fields | ||
.iter() | ||
.map(|f| { | ||
let field = f.field().clone().with_nullable(true); | ||
if let Some(q) = f.qualifier() { | ||
DFField::from_qualified(q, field) | ||
} else { | ||
DFField::from(field) | ||
comphead marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
}) | ||
.collect(); | ||
left_fields | ||
.iter() | ||
.chain(&right_fields_nullable) | ||
.cloned() | ||
.collect() | ||
} | ||
JoinType::LeftSemi | JoinType::LeftAnti => { | ||
// Only use the left side for the schema | ||
left.fields().clone() | ||
left_fields.clone() | ||
} | ||
JoinType::RightSemi | JoinType::RightAnti => { | ||
// Only use the right side for the schema | ||
right.fields().clone() | ||
right_fields.clone() | ||
} | ||
}; | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be great to add tests like this for Right, Full, and all the other flavors of joins.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#5748 (comment)