feat(rust): add driver manager and ability to expose Rust driver as C driver #1496
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alexandreyc
requested review from
lidavidm,
zeroshade and
wjones127
as code owners
alexandreyc
force-pushed
the
rust-implement-module
branch
from
248af16
to
5be1d61
Compare
alexandreyc
force-pushed
the
rust-implement-module
branch
from
fca997a
to
7e20e56
Compare
alexandreyc
force-pushed
the
rust-implement-module
branch
from
7e20e56
to
1ff35aa
Compare
wip: add implement module fix: implement drop for FFI_AdbcError fix: address lifetime of nullable strings passed in ffi wip: implement get_info utilities feat: add better API for database objects feat: export objects collection refactor: remove tabular generic refactor: use concrete error type refactor: make less ffi-centric
alexandreyc
force-pushed
the
rust-implement-module
branch
from
1ff35aa
to
1ee7b6d
Compare
|
Close in favor of #1725 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hello world!
Here is a PR that rebases the work done by @wjones127 in #446 and #416 to implement the driver manager and add the ability to expose native Rust driver as C API driver.
As of now, I've ensured that the code compiles, and the tests pass — excluding one, which I've temporarily ignored due to uncertainty about the logic. Additionally, I've made several minor improvements throughout.
I decided to remove async methods from core traits because, at this stage, it seemed to introduce unnecessary complexity without tangible benefits, especially since the C API is synchronous. Feel free to express your thoughts if you disagree :)
While the code has reached a functional state, I believe it could benefit from a thorough review, particularly in terms of memory and thread safety. I am currently integrating it with my application and anticipate uncovering any potential bugs (or confirming their absence).
I'm aware of the progress made by @mbrobbel in issue #1326, but I find it more reasonable to merge this PR first.