Add translations for base::difftime(), and clock functions add_days(), add_years(), date_build(), get_year(), get_month(), get_day()
#1357
Conversation
|
Yes, this what I was thinking 😄 Tagging @DavisVaughan in case he has thoughts too. |
|
Seems useful to me! |
R/backend-mssql.R
Outdated
|
|
||
| # clock --------------------------------------------------------------- | ||
| add_days = function(x, n, ...) { | ||
| rlang::check_dots_empty(...) |
There was a problem hiding this comment.
You can omit the rlang namespace. The rlang package is so commonly used that it is usually imported.
And you either do check_dots_empty() or check_dots_empty0(...) but you must not pass ... to check_dots_empty().
| rlang::check_dots_empty(...) | |
| check_dots_empty() |
|
@ablack3 Do you want to finish the PR? 😄 I'm happy to merge afterwards. |
Yes, I'll finish it this week. Is there a clock function for calculating the difference between two dates that we can translate to Should we use |
|
Yeah, I think |
|
@ablack3 as you're working on this, it would be super useful if you'd take some notes so we could document the process of adding date-time wrappers for other backends (and then we could provide more expectations about how to express date time manipulations in R code for the user) |
Yes of course. I'll write some documentation and share it for feedback. It would be nice to have one set of date manipulation functions that translate to many backends. |
|
Ok I think this PR is ready. I would expect that you'll have some recommendations so please let me know if you suggested edits to the PR. I will also add similar translations in the bigrquery package and duckdb package in separate PRs. One question, why the backends for bigquery and duckdb are not in the dbplyr package? I still need to write up the documentation Hadley asked for, possibly in an issue on dplyr. |
ablack3
changed the title
R/backend-postgres.R
Outdated
| }, | ||
| add_years = function(x, n, ...) { | ||
| check_dots_empty() | ||
| sql_expr((!!x + !!n%*INTERVAL%'1 year')) |
There was a problem hiding this comment.
@mgirlich this works but is very awkward. Is there a better way to get the correct SQL here?
This is what I want to write but it is not a valid R expression
sql_expr((!!x + !!n * INTERVAL'1 year'))
The SQL should look like
(`column_name` + 2 * INTERVAL'1 year')
There was a problem hiding this comment.
@hadley is there a better way to write this expression? !!x + !!n%*INTERVAL%'1 year' seems strange but it's the only way I could come up with that worked.
There was a problem hiding this comment.
I'd probably use glue_sql2() since that's SQL expression is distant from any R equivalent.
There was a problem hiding this comment.
Ok I tried glue_sql2() instead. Hopefully I used it correctly. I'm using sql_current_con() as the first argument.
| } | ||
|
|
||
| if (units[1] != "days") { | ||
| cli::cli_abort('The only supported value for {.arg units} on SQL backends is "days"') |
There was a problem hiding this comment.
I thought we could just support the 'days' unit to start. Keep it simple.
ablack3
changed the title
difftime, clock::add_days, and clock::add_years
…get_month, get_day.
Ok I added a few more translations and added a news bullet. Thank you! |
ablack3
changed the title
difftime, clock::add_days, and clock::add_years base::difftime(), and clock functions add_days(), add_years(), date_build(), get_year(), get_month(), get_day()
@hadley, @mgirlich. Here is what I'm thinking for two of the clock functions. Once I learn how to make well formed PRs I can add more but I wanted to see what you think about this one first. Can you review and discuss or make suggestions? Thanks!