Skip to content
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

Support join_by() #1074

Merged
merged 31 commits into from
Feb 13, 2023
Merged

Support join_by() #1074

merged 31 commits into from
Feb 13, 2023

Conversation

mgirlich
Copy link
Collaborator

@mgirlich mgirlich commented Dec 9, 2022

Support for the awesome new join_by() clause 😄

@hadley hadley mentioned this pull request Dec 12, 2022
28 tasks
R/join-by-compat.R Outdated Show resolved Hide resolved
@mgirlich mgirlich marked this pull request as ready for review January 31, 2023 15:22
@mgirlich mgirlich requested a review from hadley January 31, 2023 15:34
@mgirlich mgirlich added this to the 2.3.1 milestone Feb 2, 2023
@mgirlich mgirlich mentioned this pull request Feb 10, 2023
19 tasks
R/backend-sqlite.R Outdated Show resolved Hide resolved
check_dots_empty0(...)

if (!is.character(vars)) {
message <- glue("Join columns in `{input}` must be character vectors.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When does this error arise? Is it an internal error?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably, it is an internal error. I basically only copied over the file from dplyr https://github.com/tidyverse/dplyr/blob/main/R/join-cols.R.

R/join-cols-compat.R Show resolved Hide resolved
R/lazy-join-query.R Outdated Show resolved Hide resolved
R/verb-joins.R Show resolved Hide resolved
return(x)
check_join_by_supported <- function(by, call = caller_env()) {
if (any(by$filter != "none")) {
cli_abort("Rolling joins aren't supported on database backends.", call = call)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems likely we'll need to relax this on a per backend basis at some point, but I don't think we need to do it prospectively.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there are many backends that support ASOF style joins though. None of the common ones like Postgres, SQLite, MS SQL Server, etc

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But at least there are usually some workarounds to get the same results. In case I need a rolling join in the database I might just implement the workaround in dbplyr 😄

tests/testthat/test-verb-joins.R Outdated Show resolved Hide resolved
tests/testthat/test-verb-joins.R Show resolved Hide resolved
@@ -284,7 +284,8 @@ sql_join_tbls <- function(con, by, na_matches = "never") {
sql_expr_matches(sql(lhs[[i]]), sql(rhs[[i]]), con = con)
})
} else {
compare <- paste0(lhs, " = ", rhs)
by$condition[by$condition == "=="] <- "="
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only real change to the SQL generation right? Pretty much everything else is improved input validation and new handling of output columns?

@mgirlich mgirlich merged commit d3fb399 into main Feb 13, 2023
@mgirlich mgirlich deleted the join-by branch February 13, 2023 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants