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: C++ Client Helpers #2274

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

paleolimbot
Copy link
Member

@paleolimbot paleolimbot commented Oct 23, 2024

Just a preliminary go at #598!

This PR implements a C++ wrapper around the C API to cut down on some of the verbosity of using ADBC from C++ and ensure the assumptions made by drivers (e.g., that parent objects are valid). One of the difficulties of building an ADBC driver is testing it...the validation library helps here but is still rather verbose. I'm hoping this will help make it easier to write great tests for new drivers!

Current syntax:

#include "driver/framework/client.h"

Driver driver;
UNWRAP_STATUS(driver.Load(...));

UNWRAP_RESULT(Database database = driver.NewDatabase());
UNWRAP_RESULT(Connection connection = database.NewConnection());
UNWRAP_RESULT(Statement statement = connection.NewStatement());

UNWRAP_STATUS(statement.SetSqlQuery("SELECT * from foofy"));
UNWRAP_RESULT(StatementStream stream, statement.ExecuteQuery());

struct ArrowArrayStream cstream;
stream.Export(&cstream);

cstream.release(&cstream);
UNWRAP_STATUS(statement.Release());
UNWRAP_STATUS(connection.Release());
UNWRAP_STATUS(database.Release());
UNWRAP_STATUS(driver.Release());

The implementation is probably more verbose than it has to be but has some nice properties:

  • Doesn't depend on the AdbcXXX() symbols (so it can be used by a driver like DuckDB and/or a future GDAL to federate from ADBC sources while also being an ADBC driver itself)
  • Doesn't segfault (or it's not supposed to!) if you release a parent structure and subsequently interact with a child (instead you (should) get an error telling you which level of object is no longer valid). This might have performance implications since it basically checks for validity all the way back up to the driver on every call (even down to the stream's get_next()!)
  • Warns you (or can be configured to warn you) if you don't explicitly release everything exactly once (and tells you what you forgot to not explicitly release instead before leaking or instead of crashing)

Work in progress!

@paleolimbot
Copy link
Member Author

@lidavidm This is still very early but I'd like to make sure it's headed in vaguely the right direction before I get too far!

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.

1 participant