-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
feat: Enable joins between compatible differing numeric key columns #20332
feat: Enable joins between compatible differing numeric key columns #20332
Conversation
Nice, I think we should follow up with a |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #20332 +/- ##
==========================================
+ Coverage 79.49% 79.50% +0.01%
==========================================
Files 1569 1569
Lines 218634 218782 +148
Branches 2462 2462
==========================================
+ Hits 173799 173953 +154
+ Misses 44267 44261 -6
Partials 568 568 ☔ View full report in Codecov by Sentry. |
let key_cols_coalesced = | ||
options.args.should_coalesce() && matches!(&options.args.how, JoinType::Full); | ||
|
||
if key_cols_coalesced { |
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.
I've made it so that we maintain the input column types in the output for all cases except for full-join with coalesce=True. Alternatively we could also just always use the supertype in the result.
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.
I think that what you did now is correct. 👍
Joins between keys with differing numeric types now succeed if there is a compatible supertype to which both can be casted to without loss of information.
Fixes #15338