Skip to content

GH-46098: [C++] Arrow Flight SQL ODBC layer #46099

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.

Sign up for GitHub

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

Jump to bottom
Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

GH-46098: [C++] Arrow Flight SQL ODBC layer #46099

wants to merge 1 commit into from
+1,116 −83

Conversation

alinaliBQ
Copy link
Contributor

@alinaliBQ alinaliBQ commented Apr 10, 2025 •
edited
Loading

Rationale for this change

  • Build initial ODBC DLL from flightsql and odbcabstraction component.

What changes are included in this PR?

  • Add handle allocation for environment and connection.
  • Add environment attribute setting

Are these changes tested?

Yes

Are there any user-facing changes?

N/A

This PR addresses the following issues:

@github-actions GitHub Actions
Copy link

⚠️ GitHub issue #46098 has been automatically assigned in GitHub to PR creator.

@alinaliBQ alinaliBQ changed the title GH-46098: [C++] Initial handle allocation in ODBC Layer GH-46098: [C++] environment & connection handle allocation in ODBC Layer Apr 10, 2025
jduo
jduo reviewed Apr 10, 2025
View reviewed changes
jduo
jduo reviewed Apr 10, 2025
View reviewed changes
jduo
jduo reviewed Apr 10, 2025
View reviewed changes
@alinaliBQ
Copy link
Contributor Author

@aiguofer I have opened a draft PR for the ODBC layer.

@alinaliBQ alinaliBQ changed the title GH-46098: [C++] environment & connection handle allocation in ODBC Layer GH-46098: [C++] initial connection in ODBC layer May 2, 2025
@alinaliBQ
Copy link
Contributor Author

@zeroshade , @assignUser , @ianmcook Follow-up on the May-07 Arrow meeting, this is the draft PR that Rob and I have for creating the initial connection in the ODBC layer. It is based on top of @jbonofre 's PR#40939. Some changes in this PR are planned to be moved to PR#40939, such as the vcpkg.json update.

alinaliBQ
alinaliBQ commented May 7, 2025
View reviewed changes
@jbonofre
Copy link
Member

jbonofre commented May 7, 2025

@alinaliBQ why not just using the original PR/branch ? Just to not loose changes/comments.

@alinaliBQ
Copy link
Contributor Author

alinaliBQ commented May 7, 2025 •
edited
Loading

@alinaliBQ why not just using the original PR/branch ? Just to not loose changes/comments.

Yup you're right, for flightsql-odbc contribution, folks should use the #40939 PR, and that PR has higher priority for review than #46099. After #40939 PR is merged, we can go back to look at this #46099 PR.

@github-actions github-actions bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels May 7, 2025
@alinaliBQ
Copy link
Contributor Author

Hello @lidavidm , this is the draft PR that is based on top of @jbonofre 's #40939. It's still a draft PR but just wanted to give you a heads up

@kou kou mentioned this pull request May 13, 2025
Merged
@alinaliBQ
Copy link
Contributor Author

alinaliBQ commented May 16, 2025 •
edited
Loading

Hi @lidavidm @kou and other interested maintainers, please let me know if you prefer the Arrow Flight SQL ODBC contribution PRs like this one to be split into smaller PRs. It would be nice to know in advance so my team can prepare for the split, thank you!

@kou
Copy link
Member

kou commented May 17, 2025

Yes. I prefer smaller PRs to larger PRs because smaller PRs are easier to review than larger PRs.

(BTW, I'm not sure whether this PR is a smaller PR or not...)

@lidavidm
Copy link
Member

Yes, smaller PRs would be preferable. (To be frank, I have yet to actually review the first PR in full, but I'd rather get the paperwork done and get it merged ASAP, then we can go over it in detail afterwards.)

@alinaliBQ
Copy link
Contributor Author

@kou @lidavidm Ok sounds good! I will rebase Bit-Quill:apache-odbc on top of apache:main branch after #40939 is merged, so the size of this PR will be smaller compared to what it shows right now. Then I plan to open new smaller PRs to split this PR.

@alinaliBQ alinaliBQ changed the title GH-46098: [C++] initial connection in ODBC layer GH-46098: [C++] Arrow Flight SQL ODBC layer May 22, 2025
@jbonofre @alinaliBQ
Add Arrow Flight SQL ODBC driver
c9c48e9 
Co-authored-by: rscales <[email protected]>

 Add initial framework for odbc dll

- Add ARROW_FLIGHT_SQL_ODBC option. If we set `ARROW_FLIGHT_SQL_ODBC=ON`, the flightsql odbc folder will be built
- Add odbc api layer for entry_point.cc
- builds odbc dll file, with ODBC APIs exported in odbc.def

Address James' comments

Fix `odbcabstraction` build errors and partially fix `flightsql_odbc` errors

Fix boost-variant not found error
- Adding dependencies from odbc/vcpkg.json to cpp/vcpkg.json

- Fix whereami.cc and .h dependency; ported lates code

Update whereami.cc

- use `long` instead of `int64`. Fixed namespace issues.

- PR CI fix: Add `parquet-testing` back

Partial build fix for `flight_sql` folder

- Replaced `namespace arrow` and `namespace odbcabstraction` with `using namespace ...`

- fix flight_sql_connection.cc

Fix `util::nullopt` to use `std::nullopt`

- fix std::optional

- fix BufferReader

- Fix GetSchema

- fix json_converter.cc

- partial fix configuration.h

-  partial fix get_info_cache.cc

- Fix winsock build error

- Comment out `flight_sql` files that cannot build
   - Comment out configuration and unit tests
   - Comment out get info cache and system trust store

Create initial odbc tests folder

Implement SQLAllocEnv

Fix cmake build

Implement SQLFreeEnv

Fix rest of build errors from `flightsql-odbc`

- Fix get info errors

- Fix for configuration window

  - added odbcinst library

- Fix system trust store

- unit test fixes

   - Add dependency of ARROW_COMPUTE. `arrow/compute/api.h` is used in
     `flight_sql`. Adding `ARROW_COMPUTE=ON` during build fixed run time unit
     tests failures.

Implement SQLAllocConnect and SQLFreeConnect

Fix build issue from static flight sql driver

Lint and code style fixes

Re-add deleted submodule parquet-testing

clang-format lint fix

cpplint lint fix

Exclude whereami in rat exclude list

C++/CLI lint fix

Update parquet-testing to match commit from `main`

Address Kou's comments

ODBC directory lint fixes

Catching the lint fixes outside of `flightsql-odbc` code

Fix build warnings that get treated as error

Implement SQLSetEnvAttr and SQLGetEnvAttr

Implement use of ExecuteWithDiagnostics

Doxygen Error Fixes and Address comments from Kou and James

Address comments from Kou

- Updates License.txt
- Update cmake toolchain
- Move whereami to `vendored`
- Use string_view instead of NOLINT std::string

Remove `whereami.cc` from arrow util build

We are building whereami.cc as part of odbc

Fix include headers to replace <> with ""

Address comments from James

Implement SQLGetDiagField
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jduo jduo jduo left review comments

@lidavidm lidavidm Awaiting requested review from lidavidm lidavidm will be requested when the pull request is marked ready for review lidavidm is a code owner

Assignees
No one assigned
Labels
awaiting committer review Awaiting committer review Component: C++ Component: FlightRPC
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@alinaliBQ @jbonofre @kou @lidavidm @jduo