-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Add Arrow Flight SQL ODBC driver #40939
base: main
Are you sure you want to change the base?
Conversation
Thanks for opening a pull request! If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project. Then could you also rename the pull request title in the following format?
or
In the case of PARQUET issues on JIRA the title also supports:
See also: |
@ianmcook @lidavidm @assignUser as discussed during the community meetings, here's the ODBC driver PR draft. This PR includes:
I'm still working on:
I will also create a GH Issue to track this addition. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this @jbonofre !
Some of the code in odbc_impl should be updated to use the same naming conventions as the arrow project (eg ODBCConnection -> odbc_connection).
This covers the flightsql-odbc repo, but there is additional code needed for the ODBC entry points to build a functional driver. That was in warpdrive, but that code isn't Apache-compatible.
Need to add the license for whereami to the Arrow project itself.
Are the separate brewfile and vcpkg.json necessary or can this utilize the ones already at the top-level?
Should the top-level CMakeLists now include the driver build? Or is that on hold as part of the donation process?
void Create(); | ||
|
||
/** | ||
* @copedoc ignite::odbc::system::ui::CustomWindow::OnCreate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These @copedoc tags are causing doxygen failures in CI:
https://github.com/apache/arrow/actions/runs/8518121914/job/23329735731?pr=40939#step:6:3040
I fear I can't really help on that front but once that's done I'd be happy to continue working on the CMake! |
Hi @jduo !
Yes, I'm doing that (that's why the PR is still a draft).
Yes, locally I fixed/updated it, but I'm looking for alternative.
I'm checking if we can't find an alternative, else I will update
Yes, that's the plan, I'm doing in two steps: fixing the build and integrating in the Arrow ecosystem.
Agree, same the same as for I will push new changes to this PR and "remove" the draft state as soon as it's clean to review (build ok, etc). |
No worries, I'm working on it :) I will keep you posted when good for review (when I will remove the draft flag). |
There was another group that did some work to build a full driver from flightsql-odbc. I haven't seen it though, but they've mentioned it here: |
Following up here - do you want any help? |
@jbonofre checking in on this. Were you still continuing work on this? Did you need help? |
@jduo yes, I'm back on it (sorry I was busy on Iceberg and ASF stuff). Let me do a new rebase/build/update and I will ping you for review/help. Thanks ! And sorry again for delay :/ |
No worries! Glad to see you back around :) |
Hey @jbonofre we're excited about the AFS ODBC driver! We want to start using it as soon as we can for a project we're working on and was wondering whether you had a sense of timing where it might be ready to be in someone's hands for development. We would be happy to be early adopters :) |
Thanks @jbonofre . We're around if you need assistance moving this forward. |
Hi @jbonofre , just checking in on if we can help to move this forward. |
Hey @jduo |
I'm curious, does this version of the driver allow passing arbitrary parameters? I tried using the Dremio ODBC driver but it can't seem to be able to set arbitrary parameters. I have the following in a custom PowerBI connector:
But i get the following error: For our use-case we need to be able to pass various parameters as extra headers, which is supported by both ADBC and JDBC. |
Last I tried, it supported this. I believe it lower-cases headers though (the C++ grpc client itself requires this) -- is your server looking for environmentId case-insensitively? |
Yeah it's case-insensitive. I also tried setting |
Interesting, I do see:
Maybe there's a bug with call options not getting set correctly for the HANDSHAKE request? I remember that being an issue with the JDBC driver: #33946 |
This is actually part of the HTTP/2 spec. |
Ok, I think I know what it is... |
960ea1b
to
9a7edb7
Compare
9a7edb7
to
4dd325d
Compare
4dd325d
to
7667e55
Compare
Rationale for this change
An ODBC driver uses the Open Database Connectivity (ODBC) interface that allow applications to access data in DBMS like environment using SQL.
As Arrow Flight provides JDBC and ADBC drivers, similarly Arrow Flight can provide ODBC driver.
What changes are included in this PR?
This PR adds an ODBC driver implementation.
Are these changes tested?
This ODBC driver is coming from a production system (SGA) that is tested/ran for a while.
Are there any user-facing changes?
No change, but new user option to use Arrow Flight.