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

feat(r): Add adbc_simulate_dbi() S3 generic #982

Closed
wants to merge 7 commits into from

Conversation

paleolimbot
Copy link
Member

@paleolimbot paleolimbot commented Aug 17, 2023

This PR adds an S3 generic that dispatches on an adbc_connection or subclass named adbc_simulate_dbi(), inspired by dbplyr::simulate_dbi(). This is primarily to help wrappers like adbi and/or dbplyr integrate ADBC drivers into the R database ecosystem. Thanks to @lidavidm for the idea of using the vendor code from adbc_connection_get_info(): this allows something like the FlightSQL driver to wrap a SQLite database and still get a basic level of integration into the R database ecosystem.

An early version of this PR also provided dbplyr wrappers; that dispatched on the simulated DBI connection however, I think that is outside the scope of the adbcdrivermanager (it would be better provided by adbi, which will be the package that provides the necessary wrappers around things like DBI::dbExecute()).

@krlmlr do you think this will provide enough information to be useful (while not getting too deeply into the SQL generation game)?

@paleolimbot paleolimbot changed the title feat(r): Add basic dbplyr integration feat(r): Add adbc_simulate_dbi() S3 generic Aug 17, 2023
@paleolimbot paleolimbot marked this pull request as ready for review August 17, 2023 18:34
Copy link
Collaborator

@krlmlr krlmlr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. From looking at it, I'm not exactly sure how to use it in adbi. Perhaps we can discuss today.

CC @nbenn.

adbc_simulate_dbi.default <- function(connection, method = NULL, ...) {
vendor_name <- tryCatch({
# 0L == ADBC_INFO_VENDOR_NAME
with_adbc(info <- adbc_connection_get_info(connection, 0L), {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should there be a local_adbc() ?

# under the License.

#' Simulate an equivalent DBI connection
#'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add details on how to use it here, or in a separate vignette?

@paleolimbot
Copy link
Member Author

Closing in favour of implementing the quote literal + quote string generics directly!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants