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

new param timezone_out to control the timezone returned to R #294

Merged
merged 11 commits into from Nov 8, 2019

Conversation

shrektan
Copy link
Collaborator

@shrektan shrektan commented Aug 26, 2019

Closes #275

This PR allows user to define a new param timezone_out so that the timezone of the returned object can be local timezone or whatever other than UTC.

I believe it is very convinient for many users because usually the timezones on the local machine and the server are the same. By forcing the timezone to be UTC leads to sort-of confusing for users, especially those who are not familiar with timezone things.

An Example

Note this result now displays the same as what's in SQL. But without this functionality, the user will get "2018-12-31 20:00:00 UTC" and have to convert it to "Asia/Shanghai" later by themselves, which is verbose and inconvinient.

con <- DBI::dbConnect(
  odbc::odbc(),
  server = '#hide#', database = '#hide#', uid = '#hide#', pwd = '#hide#',
  encoding = 'GB2312', driver = 'SQL Server', port = #hide#,
  timezone = 'Asia/Shanghai', timezone_out = 'Asia/Shanghai'
)
out <- DBI::dbGetQuery(con, "select cast('2019-01-01 04:00:00' as datetime) as datetime")
str(out)
#> 'data.frame':    1 obs. of  1 variable:
#>  $ datetime: POSIXct, format: "2019-01-01 04:00:00"
attributes(out$datetime)  
#> $class
#> [1] "POSIXct" "POSIXt" 
#> 
#> $tzone
#> [1] "Asia/Shanghai"
DBI::dbDisconnect(con)

Created on 2019-08-26 by the reprex package (v0.2.1)

Copy link
Member

@krlmlr krlmlr left a comment

Choose a reason for hiding this comment

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

Thanks, looks good! I believe this is the only safe way to implement this. Ideally we'd use timezone_out = timezone as default but this seems very risky for scripts that already set a timezone argument.

src/odbc_result.cpp Outdated Show resolved Hide resolved
@shrektan
Copy link
Collaborator Author

shrektan commented Oct 9, 2019

@krlmlr It should be correct now!

db_tz <- 'Asia/Shanghai' 
user_tz <- 'America/Los_Angeles'
con <- DBI::dbConnect(
  odbc::odbc(),
  server = '127.0.0.1', database = 'tempdb', uid = 'sa', pwd = 'Abc_123_Bcd',
  encoding = 'GB2312', driver = 'ODBC Driver 13 for SQL Server', port = 1433,
  timezone = db_tz, timezone_out = user_tz
)
out <- DBI::dbGetQuery(con, "select cast('2019-01-01 04:00:00' as datetime) as datetime, 
                       cast('2019-01-01' as date) as date")
str(out)
#> 'data.frame':    1 obs. of  2 variables:
#>  $ datetime: POSIXct, format: "2018-12-31 12:00:00"
#>  $ date    : Date, format: "2019-01-01"
attributes(out$datetime)
#> $class
#> [1] "POSIXct" "POSIXt" 
#> 
#> $tzone
#> [1] "America/Los_Angeles"
lubridate::with_tz(out$datetime, db_tz)
#> [1] "2019-01-01 04:00:00 CST"
print(out$date)
#> [1] "2019-01-01"
DBI::dbDisconnect(con)

Created on 2019-10-10 by the reprex package (v0.3.0)

@shrektan
Copy link
Collaborator Author

The failed check is only happened on R-devel and is caused by a warning "checking top-level files ... WARNING", which seems not related to this PR.

Moreover, just to note that now it's possible to have an MS SQL Server inside of a docker container, making the tests more easily. See quickstart-install-connect-docker for details. 😃

@jimhester
Copy link
Contributor

Nice, that is useful!

@jimhester jimhester merged commit fc734a9 into r-dbi:master Nov 8, 2019
@jimhester
Copy link
Contributor

Awesome, great work!

Thanks a bunch!

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.

[FR] be able to control the returned timezone
3 participants