GH-14907: [R] right_join() function does not produce the expected outcome #15077
Conversation
|
|
wjones127
left a comment
wjones127
left a comment
There was a problem hiding this comment.
Thanks for creating this. This works, but I think the underlying is cause is elsewhere, so might be better to fix there.
r/tests/testthat/test-dplyr-join.R
Outdated
| } | ||
| }) | ||
|
|
||
| test_that("right_join correctly coalesces keys", { |
There was a problem hiding this comment.
Instead of another test, could we modify the existing left and to_join to have keys that don't fully overlap?
arrow/r/tests/testthat/test-dplyr-join.R
Lines 20 to 27 in 785ab5f
Then we get coverage from:
arrow/r/tests/testthat/test-dplyr-join.R
Lines 143 to 152 in 785ab5f
r/R/dplyr-join.R
Outdated
| # Initially keep join keys so we can coalesce them after when keep=FALSE | ||
| query <- do_join(x, y, by, copy, suffix, ..., keep = TRUE, join_type = "RIGHT_OUTER") | ||
|
|
||
| # If we are doing a right outer join and not keeping the join keys of | ||
| # both sides, we need to coalesce. Otherwise, rows that exist in the | ||
| # RHS will have NAs for the join keys. | ||
| if (!keep) { | ||
| query$selected_columns <- post_join_projection(names(x), names(y), handle_join_by(by, x, y), suffix) | ||
| } |
There was a problem hiding this comment.
This works, but it might be straightforward to fix here:
Lines 37 to 43 in 785ab5f
in the case of right join, we need to do the setdiff to produce the left_output and leave right_output as names(y).
There was a problem hiding this comment.
I tried the way you suggested, but I think I still need the post_join_projection() call; if I revert what I've done and just make the changes suggested above, I get an error in my test as the schema expects some_grouping.x and some_grouping.y columns to have been created, but they have not.
There was a problem hiding this comment.
Try this (works for me locally):
left_output <- if (!keep && join_type == "RIGHT_OUTER") {
setdiff(names(x), by)
} else {
names(x)
}
right_output <- if (keep || join_type %in% c("FULL_OUTER", "RIGHT_OUTER")) {
names(y)
} else {
setdiff(names(y), by)
}There was a problem hiding this comment.
Thanks, I was missing the !keep! from the left_output!
…ed outcome (apache#15077) * Closes: apache#14907 Authored-by: Nic Crane <thisisnic@gmail.com> Signed-off-by: Will Jones <willjones127@gmail.com>
|
Benchmark runs are scheduled for baseline = 53d73f8 and contender = c45ce81. c45ce81 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
|
['Python', 'R'] benchmarks have high level of regressions. |
3 participants
Uh oh!
There was an error while loading. Please reload this page.