error handling

r/adbcdrivermanager/R/error.R

@@ -42,8 +42,12 @@ adbc_error_from_array_stream <- function(stream) {
   .Call(RAdbcErrorFromArrayStream, stream)
 }
 
-adbc_allocate_error <- function(shelter = NULL) {
-  .Call(RAdbcAllocateError, shelter)
+adbc_allocate_error <- function(shelter = NULL, use_legacy_error = NULL) {
+  if (is.null(use_legacy_error)) {
+    use_legacy_error <- getOption("adbcdrivermanager.use_legacy_error", FALSE)
+  }
+
+  .Call(RAdbcAllocateError, shelter, use_legacy_error)
 }
 
 stop_for_error <- function(status, error) {

r/adbcdrivermanager/src/driver_base.h

@@ -45,15 +45,22 @@ class Error {
   }
 
   void ToAdbc(AdbcError* adbc_error, AdbcDriver* driver = nullptr) {
-    auto error_owned_by_adbc_error = new Error(message_, details_);
-    adbc_error->message = const_cast<char*>(error_owned_by_adbc_error->message_.c_str());
-    adbc_error->private_data = error_owned_by_adbc_error;
-    adbc_error->private_driver = driver;
-    adbc_error->vendor_code = ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA;
-    for (size_t i = 0; i < 5; i++) {
-      adbc_error->sqlstate[i] = error_owned_by_adbc_error->sql_state_[i];
+    if (adbc_error->vendor_code == ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA) {
+      auto error_owned_by_adbc_error = new Error(message_, details_);
+      adbc_error->message =
+          const_cast<char*>(error_owned_by_adbc_error->message_.c_str());
+      adbc_error->private_data = error_owned_by_adbc_error;
+      adbc_error->private_driver = driver;
+    } else {
+      adbc_error->message = reinterpret_cast<char*>(std::malloc(message_.size() + 1));
+      if (adbc_error->message != nullptr) {
+        memcpy(adbc_error->message, message_.c_str(), message_.size() + 1);
+      }
     }
 
+    for (size_t i = 0; i < 5; i++) {
+      adbc_error->sqlstate[i] = sql_state_[i];
+    }
     adbc_error->release = &CRelease;
   }
 
@@ -75,8 +82,13 @@ class Error {
   }
 
   static void CRelease(AdbcError* error) {
-    auto error_obj = reinterpret_cast<Error*>(error->private_data);
-    delete error_obj;
+    if (error->vendor_code == ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA) {
+      auto error_obj = reinterpret_cast<Error*>(error->private_data);
+      delete error_obj;
+    } else {
+      std::free(error->message);
+    }
+
     std::memset(error, 0, sizeof(AdbcError));
   }
 };

r/adbcdrivermanager/src/error.cc

@@ -34,15 +34,17 @@ static void finalize_error_xptr(SEXP error_xptr) {
   adbc_xptr_default_finalize<AdbcError>(error_xptr);
 }
 
-extern "C" SEXP RAdbcAllocateError(SEXP shelter_sexp) {
+extern "C" SEXP RAdbcAllocateError(SEXP shelter_sexp, SEXP use_legacy_error_sexp) {
+  bool use_legacy_error = adbc_as_bool(use_legacy_error_sexp);
+
   SEXP error_xptr = PROTECT(adbc_allocate_xptr<AdbcError>(shelter_sexp));
   R_RegisterCFinalizer(error_xptr, &finalize_error_xptr);
 
   AdbcError* error = adbc_from_xptr<AdbcError>(error_xptr);
-  error->message = nullptr;
-  error->vendor_code = 0;
-  memset(error->sqlstate, 0, sizeof(error->sqlstate));
-  error->release = nullptr;
+  *error = ADBC_ERROR_INIT;
+  if (use_legacy_error) {
+    error->vendor_code = 0;
+  }
 
   UNPROTECT(1);
   return error_xptr;

r/adbcdrivermanager/src/init.c

@@ -23,7 +23,7 @@
 SEXP RAdbcLogDriverInitFunc(void);
 SEXP RAdbcMonkeyDriverInitFunc(void);
 SEXP RAdbcVoidDriverInitFunc(void);
-SEXP RAdbcAllocateError(SEXP shelter_sexp);
+SEXP RAdbcAllocateError(SEXP shelter_sexp, SEXP use_legacy_error_sexp);
 SEXP RAdbcErrorProxy(SEXP error_xptr);
 SEXP RAdbcErrorFromArrayStream(SEXP stream_xptr);
 SEXP RAdbcStatusCodeMessage(SEXP status_sexp);
@@ -103,7 +103,7 @@ static const R_CallMethodDef CallEntries[] = {
     {"RAdbcLogDriverInitFunc", (DL_FUNC)&RAdbcLogDriverInitFunc, 0},
     {"RAdbcMonkeyDriverInitFunc", (DL_FUNC)&RAdbcMonkeyDriverInitFunc, 0},
     {"RAdbcVoidDriverInitFunc", (DL_FUNC)&RAdbcVoidDriverInitFunc, 0},
-    {"RAdbcAllocateError", (DL_FUNC)&RAdbcAllocateError, 1},
+    {"RAdbcAllocateError", (DL_FUNC)&RAdbcAllocateError, 2},
     {"RAdbcErrorProxy", (DL_FUNC)&RAdbcErrorProxy, 1},
     {"RAdbcErrorFromArrayStream", (DL_FUNC)&RAdbcErrorFromArrayStream, 1},
     {"RAdbcStatusCodeMessage", (DL_FUNC)&RAdbcStatusCodeMessage, 1},

r/adbcdrivermanager/tests/testthat/test-error.R

@@ -37,7 +37,6 @@ test_that("error allocator works", {
   expect_identical(length(err), 4L)
   expect_identical(names(err), c("message", "vendor_code", "sqlstate", "details"))
   expect_null(err$message)
-  expect_identical(err$vendor_code, 0L)
   expect_identical(err$sqlstate, as.raw(c(0x00, 0x00, 0x00, 0x00, 0x00)))
   expect_identical(err$details, setNames(list(), character()))
 })
@@ -60,3 +59,23 @@ test_that("stop_for_error() gives a custom error class with extra info", {
 
   expect_true(had_error)
 })
+
+test_that("void driver can report error to ADBC 1.0.0 structs", {
+  opts <- options(adbcdrivermanager.use_legacy_error = TRUE)
+  on.exit(options(opts))
+
+  had_error <- FALSE
+  tryCatch({
+    db <- adbc_database_init(adbc_driver_void())
+    adbc_database_get_option(db, "this option does not exist")
+  }, adbc_status = function(e) {
+    had_error <<- TRUE
+    expect_s3_class(e, "adbc_status")
+    expect_s3_class(e, "adbc_status_not_found")
+    expect_identical(e$error$status, 3L)
+    expect_identical(e$error$vendor_code, 0L)
+    expect_identical(e$error$details, setNames(list(), character()))
+  })
+
+  expect_true(had_error)
+})

r/adbcdrivermanager/tests/testthat/test-options.R

@@ -24,7 +24,7 @@ test_that("get/set option can roundtrip string options for database", {
 
   adbc_database_set_options(db, list("some_key" = "some value"))
   expect_identical(
-    adbc_database_get_option_bytes(db, "some_key"),
+    adbc_database_get_option(db, "some_key"),
     "some value"
   )
 })