Fixup/oracle dates (#810)

* oracle: Ability to batch-write dates and datetimes. * Add NEWS entry * fixup: remove todos * Update tests/testthat/test-driver-oracle.R Co-authored-by: Simon P. Couch <[email protected]> * code-review: Better explanation * code-review: style --------- Co-authored-by: Simon P. Couch <[email protected]>
r-dbi · Jun 17, 2024 · 3b65fb2 · 3b65fb2
1 parent 4090f0a
commit 3b65fb2
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 7 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -1,5 +1,7 @@
 # odbc (development version)
 
+* oracle: Fix writing to DATE and TIMESTAMP(n) targets using `batch_size` > 1.
+
 * SQL Server: Fix issue related to writing when using SIMBA drivers (#816).
 
 # odbc 1.5.0

diff --git a/R/dbi-table.R b/R/dbi-table.R
@@ -157,7 +157,7 @@ setMethod("dbAppendTable", "OdbcConnection",
  )
  rs <- OdbcResult(conn, sql)
 
- if (!is.null(fieldDetails) && nrow(fieldDetails) == nparam) {
+ if (!is.null(fieldDetails) && nrow(fieldDetails) <= nparam) {
  result_describe_parameters(rs@ptr, fieldDetails)
  }
 

diff --git a/R/driver-oracle.R b/R/driver-oracle.R
@@ -122,7 +122,24 @@ setMethod("odbcConnectionColumns_", c("Oracle", "character"),
  )
  }
 
- dbGetQuery(conn, query)
+ res <- dbGetQuery(conn, query)
+
+ # SQLColumns does not correctly describe buffer sizes for date/time
+ # fields ( nor does our custom query above ). We use these data
+ # as a fallback for SQLDescribeParam ( also incorrectly describes
+ # parameters for ORACLE ) when writing using prepared statements ( see
+ # `dbAppendTable` ). Fix them here. 91 and 93 are SQL_TYPE_DATE,
+ # and SQL_TYPE_TIMESTAMP, respectively in sql.h. Six and sixteen
+ # are the sizes of nanodbc::date and nanodbc::timestmp in bytes
+ # (#349, #350, #391).
+ res$data_type <- as.numeric(res$data_type)
+ isDate <- res$field.type == "DATE"
+ res$data_type[isDate] <- 91
+ res$column_size[isDate] <- 6
+ isTimestamp <- grepl("TIMESTAMP", res$field.type)
+ res$data_type[isTimestamp] <- 93
+ res$column_size[isTimestamp] <- 16
+ res
  }
 )
 

diff --git a/tests/testthat/test-driver-oracle.R b/tests/testthat/test-driver-oracle.R
@@ -1,12 +1,13 @@
 test_that("can round columns", {
  con <- test_con("ORACLE")
- # - Long/outstanding issue with batch inserting
- # date/datetime for Oracle. See for example
- # #349, #350, #391
+ # - Date does not roundtrip correctly since
+ # their DATE data-type contains hour/min/seconds.
+ # To avoid loss of precision we read it in as
+ # POSIXct.
  # - There also looks like there are issues related
- # to binary elements of size zero.
+ #  to binary elements of size zero.
  # - Finally, no boolean in Oracle prior to 23
- test_roundtrip(con, columns = c("time", "date", "datetime", "binary", "logical"))
+ test_roundtrip(con, columns = c("time", "date", "binary", "logical"))
 })
 
 test_that("can detect existence of table", {
@@ -18,3 +19,18 @@ test_that("can detect existence of table", {
  tbl2 <- local_table(con, "mtcars_test", mtcars)
  expect_true(dbExistsTable(con, tbl2))
 })
+
+test_that("Writing date/datetime with batch size > 1", {
+ # See #349, #350, #391
+ con <- test_con("ORACLE")
+
+ values <- data.frame(
+ datetime = as.POSIXct(as.numeric(iris$Petal.Length * 10), origin = "2024-01-01", tz = "UTC"),
+ date = as.Date(as.numeric(iris$Petal.Length * 10), origin = "2024-01-01", tz = "UTC"),
+ integer = as.integer(iris$Petal.Width * 100),
+ double = iris$Sepal.Length,
+ varchar = iris$Species)
+ tbl <- local_table(con, "test_batched_write_w_dates", values)
+ res <- dbReadTable(con, "test_batched_write_w_dates")
+ expect_true(nrow(res) == nrow(values))
+})