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

refactor(r): Improve testing for ADBC 1.1 features in R bindings #1214

Merged
merged 43 commits into from
Oct 26, 2023
Merged
Show file tree
Hide file tree
Changes from 41 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
2c8b98a
sketch driver_base
paleolimbot Oct 19, 2023
d40d6d1
convert void to cc
paleolimbot Oct 19, 2023
951042d
comiles
paleolimbot Oct 19, 2023
30f2639
with compiling driver void
paleolimbot Oct 19, 2023
2b4848f
with connection
paleolimbot Oct 19, 2023
98ffe0c
driver that loads
paleolimbot Oct 19, 2023
c98b4f7
with void driver + passing tests
paleolimbot Oct 19, 2023
dcf5f21
simplify templating
paleolimbot Oct 19, 2023
73df45a
with options
paleolimbot Oct 19, 2023
f990879
test option getting
paleolimbot Oct 19, 2023
fa22676
move statement trampolines
paleolimbot Oct 20, 2023
71e8fb9
clean up option a little
paleolimbot Oct 20, 2023
66b0fe7
fix build
paleolimbot Oct 20, 2023
fe34077
with better separation of functions intended to be used from C++ and …
paleolimbot Oct 20, 2023
6db933e
trampolines to the driver class
paleolimbot Oct 20, 2023
5cd76a3
remove superfluous qualifiers
paleolimbot Oct 20, 2023
a04b355
more reorganizing methods
paleolimbot Oct 20, 2023
cd7867b
test error with details
paleolimbot Oct 20, 2023
8e6a59a
move key value options and tests to options files
paleolimbot Oct 20, 2023
e740c8c
handle more types in key/value options
paleolimbot Oct 20, 2023
48b7621
raw values
paleolimbot Oct 20, 2023
c98dc5b
switch on set options to set other types of options
paleolimbot Oct 20, 2023
3914930
check bytes options
paleolimbot Oct 20, 2023
5675163
test option getting/setting for int
paleolimbot Oct 20, 2023
d0c5c54
test double
paleolimbot Oct 20, 2023
60d630b
fix accidental invalidation db reinit
paleolimbot Oct 20, 2023
6dca8bc
RAT
paleolimbot Oct 20, 2023
ef0fe2f
cpplint
paleolimbot Oct 20, 2023
a8eec39
sync
paleolimbot Oct 20, 2023
753f3e9
more cpplint
paleolimbot Oct 21, 2023
ba1d690
doubles always stay doubles
paleolimbot Oct 23, 2023
97df1e2
separate error option tests
paleolimbot Oct 23, 2023
2562d80
move error methods intended to be called from C
paleolimbot Oct 23, 2023
c520a44
error handling
paleolimbot Oct 23, 2023
df4eeae
use ADBC_ERROR_INIT in radbc.cc
paleolimbot Oct 23, 2023
05601b9
remove bits that weren't tested
paleolimbot Oct 23, 2023
0a61beb
format
paleolimbot Oct 23, 2023
75e70c0
expose driver to subclasses
paleolimbot Oct 23, 2023
aff4781
document the header
paleolimbot Oct 23, 2023
1fccb0f
remove accidentally comitted vendored file
paleolimbot Oct 23, 2023
0253e41
reflow comment
paleolimbot Oct 23, 2023
8ced5b1
use array instead of string
paleolimbot Oct 26, 2023
74881c6
use `std::move()`
paleolimbot Oct 26, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions c/driver_manager/adbc_driver_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,11 @@ AdbcStatusCode StatementBind(struct AdbcStatement*, struct ArrowArray*,
return ADBC_STATUS_NOT_IMPLEMENTED;
}

AdbcStatusCode StatementBindStream(struct AdbcStatement*, struct ArrowArrayStream*,
struct AdbcError* error) {
return ADBC_STATUS_NOT_IMPLEMENTED;
}

AdbcStatusCode StatementCancel(struct AdbcStatement* statement, struct AdbcError* error) {
return ADBC_STATUS_NOT_IMPLEMENTED;
}
Expand Down Expand Up @@ -1666,6 +1671,7 @@ AdbcStatusCode AdbcLoadDriverFromInitFunc(AdbcDriverInitFunc init_func, int vers
CHECK_REQUIRED(driver, StatementNew);
CHECK_REQUIRED(driver, StatementRelease);
FILL_DEFAULT(driver, StatementBind);
FILL_DEFAULT(driver, StatementBindStream);
FILL_DEFAULT(driver, StatementGetParameterSchema);
FILL_DEFAULT(driver, StatementPrepare);
FILL_DEFAULT(driver, StatementSetOption);
Expand Down
6 changes: 6 additions & 0 deletions go/adbc/drivermgr/adbc_driver_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,11 @@ AdbcStatusCode StatementBind(struct AdbcStatement*, struct ArrowArray*,
return ADBC_STATUS_NOT_IMPLEMENTED;
}

AdbcStatusCode StatementBindStream(struct AdbcStatement*, struct ArrowArrayStream*,
struct AdbcError* error) {
return ADBC_STATUS_NOT_IMPLEMENTED;
}

AdbcStatusCode StatementCancel(struct AdbcStatement* statement, struct AdbcError* error) {
return ADBC_STATUS_NOT_IMPLEMENTED;
}
Expand Down Expand Up @@ -1666,6 +1671,7 @@ AdbcStatusCode AdbcLoadDriverFromInitFunc(AdbcDriverInitFunc init_func, int vers
CHECK_REQUIRED(driver, StatementNew);
CHECK_REQUIRED(driver, StatementRelease);
FILL_DEFAULT(driver, StatementBind);
FILL_DEFAULT(driver, StatementBindStream);
FILL_DEFAULT(driver, StatementGetParameterSchema);
FILL_DEFAULT(driver, StatementPrepare);
FILL_DEFAULT(driver, StatementSetOption);
Expand Down
8 changes: 6 additions & 2 deletions r/adbcdrivermanager/R/error.R
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
74 changes: 71 additions & 3 deletions r/adbcdrivermanager/R/options.R
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ adbc_database_set_options <- function(database, options) {
error <- adbc_allocate_error()
for (i in seq_along(options)) {
key <- names(options)[i]
value <- options[i]
value <- options[[i]]
status <- .Call(
RAdbcDatabaseSetOption,
database,
Expand All @@ -42,7 +42,7 @@ adbc_connection_set_options <- function(connection, options) {
error <- adbc_allocate_error()
for (i in seq_along(options)) {
key <- names(options)[i]
value <- options[i]
value <- options[[i]]
status <- .Call(
RAdbcConnectionSetOption,
connection,
Expand All @@ -62,7 +62,7 @@ adbc_statement_set_options <- function(statement, options) {
error <- adbc_allocate_error()
for (i in seq_along(options)) {
key <- names(options)[i]
value <- options[i]
value <- options[[i]]
status <- .Call(
RAdbcStatementSetOption,
statement,
Expand Down Expand Up @@ -160,3 +160,71 @@ adbc_statement_get_option_double <- function(statement, option) {
error <- adbc_allocate_error()
.Call(RAdbcStatementGetOptionDouble, statement, option, error)
}

# Ensures that options are a list of bare character, raw, integer, or double
key_value_options <- function(options) {
options <- as.list(options)

if (length(options) == 0) {
names(options) <- character()
} else if (is.null(names(options))) {
# OK to have no names, because options could contain a series of
# adbc_options() objects that will be concatenated
names(options) <- rep("", length(options))
}

out <- vector("list", 10L)
out_names <- character(10L)
n_out <- 0L

for (i in seq_along(options)) {
key <- names(options)[[i]]
item <- options[[i]]

# Skip NULL item
if (is.null(item)) {
next
}

# Append all items of an existing adbc_options
if (inherits(item, "adbc_options")) {
out <- c(out[seq_len(n_out)], item)
out_names <- c(out_names[seq_len(n_out)], names(item))
n_out <- n_out + length(item)
next
}

# Otherwise, append a single value (coercing to character if item
# is an S3 object)
if (is.object(item)) {
item <- as.character(item)
}

if (identical(key, "") || identical(key, NA_character_)) {
stop("key/value options must be named")
}

n_out <- n_out + 1L
out_names[n_out] <- key
if (is.character(item)) {
out[[n_out]] <- item
} else if (is.integer(item)) {
out[[n_out]] <- as.integer(item)
} else if (is.double(item)) {
out[[n_out]] <- as.double(item)
} else if (is.raw(item)) {
out[[n_out]] <- item
} else {
stop(
sprintf(
"Option of type '%s' (key: '%s') not supported",
typeof(item),
key
)
)
}
}

names(out) <- out_names
structure(out[seq_len(n_out)], class = "adbc_options")
}
17 changes: 0 additions & 17 deletions r/adbcdrivermanager/R/utils.R
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,6 @@
# specific language governing permissions and limitations
# under the License.

key_value_options <- function(options) {
if (!is.character(options)) {
options <- as.list(options)
options <- options[!vapply(options, is.null, logical(1))]
options <- vapply(options, as.character, character(1))
}

keys <- names(options)
if (length(options) == 0) {
names(options) <- character()
} else if (is.null(keys) || all(keys == "")) {
stop("key/value options must be named")
}

options
}

new_env <- function() {
new.env(parent = emptyenv())
}
Expand Down
Loading
Loading