make odbcDataType() respect integer64
#698
Conversation
|
With this PR: library(odbc)
library(DBI)
suppressPackageStartupMessages(library(bit64))
library(dbplyr)
odbc::odbcDataType(dbplyr::simulate_mssql(), bit64::as.integer64(1))
#> [1] "BIGINT"Created on 2023-12-19 with reprex v2.0.2 |
There was a problem hiding this comment.
My only worry with this change is that it's going to break use of integer64 with other backends because it will now hit the unsupported type clause. So I think you should update all the odbcDataType() methods to add integer64 to the switch, just using the same value as integer.
| @@ -92,6 +92,9 @@ object_type <- function(obj) { | |||
| if (is(obj, "difftime")) { | |||
| return("time") | |||
| } | |||
| if (is(obj, "integer64")) { | |||
There was a problem hiding this comment.
These should all really use inherits(), so I added to #673
Since `switch_type()` now knows about integer64, we set the mappings for `int64` to whatever they used to map to in methods where we don't want behavior to change.
|
Totally makes sense. 628879c. |
NEWS.md
Outdated
| @@ -5,6 +5,8 @@ | |||
|
|
|||
| * Use correct parent class for Oracle (#685). | |||
|
|
|||
| * The Microsoft SQL Server method for `odbcDataType()` will now return `"BIGINT"` | |||
There was a problem hiding this comment.
Conventionally we usually put new news bullets at the top. No need to change here, just remember in the future.
There was a problem hiding this comment.
Heard. Had to merge NEWS upstream anyway so just bumped that entry up.🐧
Closes #514.