-
Notifications
You must be signed in to change notification settings - Fork 48
Support both dbplyr < 2.6.0 and >= 2.6.0 with backwards compatibility #2032
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
base: main
Are you sure you want to change the base?
Changes from 4 commits
1032d26
e6834fc
2f45892
3908bf7
5f0cd3e
06dc487
14b12d3
f9dc553
787a2f5
1104cd8
e29b342
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 |
|---|---|---|
|
|
@@ -68,9 +68,27 @@ duckdb_grepl <- function(pattern, x, ignore.case = FALSE, perl = FALSE, fixed = | |
| } | ||
| } | ||
|
|
||
| # Memoised detection of dbplyr's sql_glue availability | ||
| # Returns TRUE if dbplyr >= 2.5.2 with exported sql_glue, FALSE otherwise | ||
| has_sql_glue <- local({ | ||
| result <- NULL | ||
| function() { | ||
| if (is.null(result)) { | ||
| result <<- tryCatch({ | ||
| # Check if dbplyr is available and has sql_glue exported | ||
| if (requireNamespace("dbplyr", quietly = TRUE)) { | ||
| "sql_glue" %in% getNamespaceExports("dbplyr") | ||
| } else { | ||
| FALSE | ||
| } | ||
| }, error = function(e) FALSE) | ||
| } | ||
| result | ||
| } | ||
| }) | ||
|
|
||
| duckdb_n_distinct <- function(..., na.rm = FALSE) { | ||
| sql <- pkg_method("sql", "dbplyr") | ||
| glue_sql2 <- pkg_method("glue_sql2", "dbplyr") | ||
| sql_current_con <- pkg_method("sql_current_con", "dbplyr") | ||
| check_dots_unnamed <- pkg_method("check_dots_unnamed", "rlang") | ||
|
|
||
|
|
@@ -82,22 +100,48 @@ duckdb_n_distinct <- function(..., na.rm = FALSE) { | |
| check_dots_unnamed() | ||
|
|
||
| # https://duckdb.org/docs/sql/data_types/struct.html#creating-structs-with-the-row-function | ||
| if (!identical(na.rm, FALSE)) { | ||
| if (length(list(...)) == 1L) { | ||
| # in case of only one column fall back to the "simple" version | ||
| return(glue_sql2(con, "COUNT(DISTINCT {.col {list(...)}*})")) | ||
| if (has_sql_glue()) { | ||
krlmlr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| # dbplyr >= 2.5.2: use exported sql_glue() | ||
| sql_glue <- pkg_method("sql_glue", "dbplyr") | ||
|
||
|
|
||
| if (!identical(na.rm, FALSE)) { | ||
| if (length(list(...)) == 1L) { | ||
| # in case of only one column fall back to the "simple" version | ||
| return(sql_glue(con, "COUNT(DISTINCT {.col {list(...)}*})")) | ||
| } else { | ||
| str_null_check <- | ||
| sql(paste0(paste0(list(...), " IS NOT NULL"), collapse = " AND ")) | ||
|
|
||
| return(sql_glue( | ||
| con, | ||
| "COUNT(DISTINCT row({.col {list(...)}*})) FILTER (", | ||
| str_null_check, ")" | ||
| )) | ||
| } | ||
| } else { | ||
| str_null_check <- | ||
| sql(paste0(paste0(list(...), " IS NOT NULL"), collapse = " AND ")) | ||
|
|
||
| return(glue_sql2( | ||
| con, | ||
| "COUNT(DISTINCT row({.col {list(...)}*})) FILTER (", | ||
| str_null_check, ")" | ||
| )) | ||
| return(sql_glue(con, "COUNT(DISTINCT row({.col {list(...)}*}))")) | ||
| } | ||
| } else { | ||
| return(glue_sql2(con, "COUNT(DISTINCT row({.col {list(...)}*}))")) | ||
| # dbplyr <= 2.5.1: use glue_sql2() | ||
| glue_sql2 <- pkg_method("glue_sql2", "dbplyr") | ||
|
|
||
| if (!identical(na.rm, FALSE)) { | ||
| if (length(list(...)) == 1L) { | ||
| # in case of only one column fall back to the "simple" version | ||
| return(glue_sql2(con, "COUNT(DISTINCT {.col {list(...)}*})")) | ||
| } else { | ||
| str_null_check <- | ||
| sql(paste0(paste0(list(...), " IS NOT NULL"), collapse = " AND ")) | ||
|
|
||
| return(glue_sql2( | ||
| con, | ||
| "COUNT(DISTINCT row({.col {list(...)}*})) FILTER (", | ||
| str_null_check, ")" | ||
| )) | ||
| } | ||
| } else { | ||
| return(glue_sql2(con, "COUNT(DISTINCT row({.col {list(...)}*}))")) | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
||
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.
Can this be
dbplyr::sql, is this an exported function? Can we inline it?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.
Changed to use
dbplyr::sqldirectly in commit 5f0cd3e. The function is inlined where used.