EpsilonPrime commented on code in PR #1206:
URL: https://github.com/apache/arrow-adbc/pull/1206#discussion_r1362852937
##########
r/adbcdrivermanager/src/error.cc:
##########
@@ -48,9 +48,28 @@ extern "C" SEXP RAdbcAllocateError(SEXP shelter_sexp) {
return error_xptr;
}
+static SEXP wrap_error_details(AdbcError* error) {
+ int n_details = AdbcErrorGetDetailCount(error);
+ SEXP result_names = PROTECT(Rf_allocVector(STRSXP, n_details));
+ SEXP result = PROTECT(Rf_allocVector(VECSXP, n_details));
+
+ for (int i = 0; i < n_details; i++) {
Review Comment:
From a C++ style standpoint I tend to use ++i here since i++ returns a value
and ++i won't. Sometimes this can equate into a performance improvement but I
doubt we would see it here.
##########
r/adbcdrivermanager/src/radbc.cc:
##########
@@ -319,8 +295,8 @@ extern "C" SEXP RAdbcConnectionGetTableSchema(SEXP
connection_xptr, SEXP catalog
SEXP db_schema_sexp, SEXP
table_name_sexp,
SEXP schema_xptr, SEXP
error_xptr) {
auto connection = adbc_from_xptr<AdbcConnection>(connection_xptr);
- const char* catalog = adbc_as_const_char(catalog_sexp);
- const char* db_schema = adbc_as_const_char(db_schema_sexp);
+ const char* catalog = adbc_as_const_char(catalog_sexp, true);
Review Comment:
Add `/*nullable=*/` so it's clear what's going on here?
##########
r/adbcdrivermanager/src/options.cc:
##########
@@ -0,0 +1,271 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#define R_NO_REMAP
+#include <R.h>
+#include <Rinternals.h>
+
+#include <adbc.h>
+
+#include "radbc.h"
+
+template <typename T>
+static inline T adbc_as_c(SEXP sexp);
+
+template <>
+inline const char* adbc_as_c(SEXP sexp) {
+ return adbc_as_const_char(sexp);
+}
+
+template <>
+inline int64_t adbc_as_c(SEXP sexp) {
+ return adbc_as_int64(sexp);
+}
+
+template <>
+inline double adbc_as_c(SEXP sexp) {
+ return adbc_as_double(sexp);
+}
+
+template <typename T, typename ValueT>
+SEXP adbc_set_option(SEXP obj_xptr, SEXP key_sexp, SEXP value_sexp, SEXP
error_xptr,
+ AdbcStatusCode (*SetOption)(T*, const char*, ValueT,
AdbcError*)) {
+ auto obj = adbc_from_xptr<T>(obj_xptr);
+ const char* key = adbc_as_const_char(key_sexp);
+ ValueT value = adbc_as_c<ValueT>(value_sexp);
+ auto error = adbc_from_xptr<AdbcError>(error_xptr);
+ return adbc_wrap_status(SetOption(obj, key, value, error));
+}
+
+template <typename T>
+SEXP adbc_set_option_bytes(SEXP obj_xptr, SEXP key_sexp, SEXP value_sexp, SEXP
error_xptr,
+ AdbcStatusCode (*SetOption)(T*, const char*, const
uint8_t*,
+ size_t, AdbcError*)) {
+ auto obj = adbc_from_xptr<T>(obj_xptr);
+ const char* key = adbc_as_const_char(key_sexp);
+ const uint8_t* value = RAW(value_sexp);
+ size_t value_length = Rf_xlength(value_sexp);
+ auto error = adbc_from_xptr<AdbcError>(error_xptr);
+
+ int status = SetOption(obj, key, value, value_length, error);
+ return adbc_wrap_status(status);
+}
+
+extern "C" SEXP RAdbcDatabaseSetOption(SEXP database_xptr, SEXP key_sexp, SEXP
value_sexp,
+ SEXP error_xptr) {
+ return adbc_set_option<AdbcDatabase, const char*>(database_xptr, key_sexp,
value_sexp,
+ error_xptr,
&AdbcDatabaseSetOption);
+}
+
+extern "C" SEXP RAdbcDatabaseSetOptionBytes(SEXP database_xptr, SEXP key_sexp,
+ SEXP value_sexp, SEXP error_xptr) {
+ return adbc_set_option_bytes<AdbcDatabase>(database_xptr, key_sexp,
value_sexp,
+ error_xptr,
&AdbcDatabaseSetOptionBytes);
+}
+
+extern "C" SEXP RAdbcDatabaseSetOptionInt(SEXP database_xptr, SEXP key_sexp,
+ SEXP value_sexp, SEXP error_xptr) {
+ return adbc_set_option<AdbcDatabase, int64_t>(database_xptr, key_sexp,
value_sexp,
+ error_xptr,
&AdbcDatabaseSetOptionInt);
+}
+
+extern "C" SEXP RAdbcDatabaseSetOptionDouble(SEXP database_xptr, SEXP key_sexp,
+ SEXP value_sexp, SEXP error_xptr)
{
+ return adbc_set_option<AdbcDatabase, double>(database_xptr, key_sexp,
value_sexp,
+ error_xptr,
&AdbcDatabaseSetOptionDouble);
+}
+
+extern "C" SEXP RAdbcConnectionSetOption(SEXP connection_xptr, SEXP key_sexp,
+ SEXP value_sexp, SEXP error_xptr) {
+ return adbc_set_option<AdbcConnection, const char*>(
+ connection_xptr, key_sexp, value_sexp, error_xptr,
&AdbcConnectionSetOption);
+}
+
+extern "C" SEXP RAdbcConnectionSetOptionBytes(SEXP connection_xptr, SEXP
key_sexp,
+ SEXP value_sexp, SEXP
error_xptr) {
+ return adbc_set_option_bytes<AdbcConnection>(connection_xptr, key_sexp,
value_sexp,
+ error_xptr,
&AdbcConnectionSetOptionBytes);
+}
+
+extern "C" SEXP RAdbcConnectionSetOptionInt(SEXP connection_xptr, SEXP
key_sexp,
+ SEXP value_sexp, SEXP error_xptr) {
+ return adbc_set_option<AdbcConnection, int64_t>(
+ connection_xptr, key_sexp, value_sexp, error_xptr,
&AdbcConnectionSetOptionInt);
+}
+
+extern "C" SEXP RAdbcConnectionSetOptionDouble(SEXP connection_xptr, SEXP
key_sexp,
+ SEXP value_sexp, SEXP
error_xptr) {
+ return adbc_set_option<AdbcConnection, double>(
+ connection_xptr, key_sexp, value_sexp, error_xptr,
&AdbcConnectionSetOptionDouble);
+}
+
+extern "C" SEXP RAdbcStatementSetOption(SEXP statement_xptr, SEXP key_sexp,
+ SEXP value_sexp, SEXP error_xptr) {
+ return adbc_set_option<AdbcStatement, const char*>(statement_xptr, key_sexp,
value_sexp,
+ error_xptr,
&AdbcStatementSetOption);
+}
+
+extern "C" SEXP RAdbcStatementSetOptionBytes(SEXP statement_xptr, SEXP
key_sexp,
+ SEXP value_sexp, SEXP error_xptr)
{
+ return adbc_set_option_bytes<AdbcStatement>(statement_xptr, key_sexp,
value_sexp,
+ error_xptr,
&AdbcStatementSetOptionBytes);
+}
+
+extern "C" SEXP RAdbcStatementSetOptionInt(SEXP statement_xptr, SEXP key_sexp,
+ SEXP value_sexp, SEXP error_xptr) {
+ return adbc_set_option<AdbcStatement, int64_t>(statement_xptr, key_sexp,
value_sexp,
+ error_xptr,
&AdbcStatementSetOptionInt);
+}
+
+extern "C" SEXP RAdbcStatementSetOptionDouble(SEXP statement_xptr, SEXP
key_sexp,
+ SEXP value_sexp, SEXP
error_xptr) {
+ return adbc_set_option<AdbcStatement, double>(
+ statement_xptr, key_sexp, value_sexp, error_xptr,
&AdbcStatementSetOptionDouble);
+}
+
+template <typename T, typename CharT>
+static inline SEXP adbc_get_option_bytes(SEXP obj_xptr, SEXP key_sexp, SEXP
error_xptr,
+ AdbcStatusCode (*GetOption)(T*, const
char*,
+ CharT*,
size_t*,
+
AdbcError*)) {
+ auto obj = adbc_from_xptr<T>(obj_xptr);
+ const char* key = adbc_as_const_char(key_sexp);
+ auto error = adbc_from_xptr<AdbcError>(error_xptr);
+
+ size_t length = 0;
+ int status = GetOption(obj, key, nullptr, &length, error);
+ adbc_error_stop(status, error);
+
+ SEXP result_shelter = PROTECT(Rf_allocVector(RAWSXP, length));
Review Comment:
Is the corresponding UNPROTECT handled at the end of this object's lifetime?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]