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

c,go,java: provide a driver 'framework' or 'base' instead of entirely separate drivers #924

Open
3 of 5 tasks
Tracked by #1490
lidavidm opened this issue Jul 20, 2023 · 4 comments
Open
3 of 5 tasks
Tracked by #1490

Comments

@lidavidm
Copy link
Member

lidavidm commented Jul 20, 2023

All the drivers in a language need to follow the same specifications, the same conventions, and have a lot of similar scaffolding. For maintainability's and testing's sake it may make more sense to create base drivers that handle most of the ADBC state and API boundaries and just dispatch to a vendor-specific backend to do things like query execution. We only have SQLite and PostgreSQL for now, but I expect we'll have an ODBC adapter soon. And it seems like the drivers may want more similar functionality than we expect: PostgreSQL decimals, for instance, will likely need type inference in the same way all SQLite type do, if we want to actually convert them to decimals.

It would also make it easier to integrate observability features like detailed logging, OpenTelemetry support, and so on, if we can (mostly) handle them in one place.

Admittedly, a lot of the code is actually in the query execution part, which may not be (easily) shareable. So it may not be as easy of a win as I think.

@lidavidm lidavidm added this to the ADBC Libraries 0.7.0 milestone Jul 20, 2023
@lidavidm
Copy link
Member Author

In particular, @ywc88 was trying to figure out how best to test #910. If we had a logging framework set up, I think it would have made the most sense to log executed queries/transaction state and inspect that for the unit test, which would let us directly verify driver interactions (much in the same way @paleolimbot's R design lets us validate how R API calls translate into C API calls, which is quite slick).

@paleolimbot
Copy link
Member

This would be very helpful and would also probably increase the probability that others' will write actually write drivers...the scafolding is quite verbose (for good reason...database drivers are hard and the C linking is worth it). At their core, ADBC drivers are "string in record batch stream out" and it would be nice if implementing a driver could minimally be implementing that one function with a driver init function of InitDriver<DriverClass, DatabaseClass, ConnectionClass, StatementClass>(void* raw_driver, int version).

@lidavidm
Copy link
Member Author

My problem with doing this right this moment is conflicting with the v1.1.0 work...I'll probably plan to start the refactor after 1.1.0 lands, then.

@lidavidm
Copy link
Member Author

And then adapting turbodbc to this would be a good first test.

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

No branches or pull requests

2 participants