GH-46004: [C++][FlightRPC] Enable ODBC Build In C++ Workflows #47689

alinaliBQ · 2025-10-02T21:14:30Z

Rationale for this change

Enable ODBC Build In C++ Workflows to make sure ODBC can build in CI.

What changes are included in this PR?

Make environment variable ARROW_FLIGHT_SQL_ODBC:ON recognized in C++ build.
Register ODBC driver dll

Are these changes tested?

Tested in CI

Are there any user-facing changes?

No

Extracted from PR #46099

GitHub Issue: [C++] Add ODBC build option to arrow root cmake file #46004

alinaliBQ · 2025-10-02T21:15:55Z

cpp/src/arrow/flight/sql/odbc/flight_sql/utils.h

 #include <arrow/flight/types.h>
+#include <boost/variant.hpp>


boost::variant is an existing dependency of flightsql-odbc. Our plan is to replace it with std::variant in future PRs

alinaliBQ · 2025-10-08T16:19:49Z

.github/workflows/ruby.yml

@@ -219,6 +219,7 @@ jobs:
      ARROW_DATASET: ON
      ARROW_FLIGHT: ON
      ARROW_FLIGHT_SQL: ON
+      ARROW_FLIGHT_SQL_ODBC: ON


ARROW_FLIGHT_SQL_ODBC: ON is already on cpp.yml file

https://github.com/apache/arrow/blob/66b2f4af147bf2af3077b3addf49fcf5b8629d52/.github/workflows/cpp.yml#L400

alinaliBQ · 2025-10-09T20:22:39Z

@lidavidm @kou The PR for building ODBC is ready for review. Please have a look, thanks

kou · 2025-10-10T00:35:33Z

cpp/src/arrow/flight/sql/odbc/odbc_impl/ui/custom_window.cc

  switch (msg) {
    case WM_NCCREATE: {
-      _ASSERT(lparam != NULL);
+      assert(lparam != NULL);


Can we use ARROW_DCHECK*() instead?

arrow/cpp/src/arrow/util/logging.h

Lines 90 to 125 in 5750e29

# ifdef NDEBUG

# define ARROW_DFATAL ::arrow::util::ArrowLogLevel::ARROW_WARNING

// CAUTION: DCHECK_OK() always evaluates its argument, but other DCHECK*() macros

// only do so in debug mode.

# define ARROW_DCHECK(condition) \

while (false) ARROW_IGNORE_EXPR(condition); \

while (false) ::arrow::util::detail::NullLog()

# define ARROW_DCHECK_OK(s) \

ARROW_IGNORE_EXPR(s); \

while (false) ::arrow::util::detail::NullLog()

# define ARROW_DCHECK_EQ(val1, val2) \

while (false) ARROW_IGNORE_EXPR(val1); \

while (false) ARROW_IGNORE_EXPR(val2); \

while (false) ::arrow::util::detail::NullLog()

# define ARROW_DCHECK_NE(val1, val2) \

while (false) ARROW_IGNORE_EXPR(val1); \

while (false) ARROW_IGNORE_EXPR(val2); \

while (false) ::arrow::util::detail::NullLog()

# define ARROW_DCHECK_LE(val1, val2) \

while (false) ARROW_IGNORE_EXPR(val1); \

while (false) ARROW_IGNORE_EXPR(val2); \

while (false) ::arrow::util::detail::NullLog()

# define ARROW_DCHECK_LT(val1, val2) \

while (false) ARROW_IGNORE_EXPR(val1); \

while (false) ARROW_IGNORE_EXPR(val2); \

while (false) ::arrow::util::detail::NullLog()

# define ARROW_DCHECK_GE(val1, val2) \

while (false) ARROW_IGNORE_EXPR(val1); \

while (false) ARROW_IGNORE_EXPR(val2); \

while (false) ::arrow::util::detail::NullLog()

# define ARROW_DCHECK_GT(val1, val2) \

while (false) ARROW_IGNORE_EXPR(val1); \

while (false) ARROW_IGNORE_EXPR(val2); \

while (false) ::arrow::util::detail::NullLog()

Sure, I have changed to use ARROW_DCHECK here.

kou · 2025-10-10T00:35:57Z

cpp/src/arrow/flight/sql/odbc/install/install_amd64.cmd

+	reg add "HKEY_LOCAL_MACHINE\SOFTWARE\ODBC\ODBCINST.INI\Apache Arrow Flight SQL ODBC Driver" /v Setup /t REG_SZ /d %ODBC_AMD64% /f
+	reg add "HKEY_LOCAL_MACHINE\SOFTWARE\ODBC\ODBCINST.INI\ODBC Drivers" /v "Apache Arrow Flight SQL ODBC Driver" /t REG_SZ /d "Installed" /f
+
+	IF !ERRORLEVEL! NEQ 0 (


Suggested change

IF !ERRORLEVEL! NEQ 0 (

if !ERRORLEVEL! NEQ 0 (

BTW, is !ERRORLEVEL! still non 0 when the first reg add succeeded and the last reg add failed?

!ERRORLEVEL! should be reflecting on the last command executed, so yes !ERRORLEVEL! should be non zero if last reg add failed.

kou · 2025-10-10T00:36:36Z

cpp/src/arrow/flight/sql/odbc/tests/install_odbc.cmd

Do we need _amd64 in file name?

It seems that this doesn't have any amd64 specific part.

Yea that's fair, I have renamed to install_odbc

cpp/src/arrow/flight/sql/odbc/odbc_impl/system_trust_store.h

cpp/src/arrow/flight/sql/odbc/CMakeLists.txt

.github/workflows/cpp.yml

kou · 2025-10-10T00:46:49Z

.github/workflows/cpp.yml

+      - name: Register Flight SQL ODBC Driver
+        shell: cmd
+        run: |
+          call "cpp\src\arrow\flight\sql\odbc\install\install_amd64.cmd" ${{github.workspace}}\build\cpp\%ARROW_BUILD_TYPE%\libarrow_flight_sql_odbc.dll


cpp\src\arrow\flight\sql\odbc\script\ not ...\install\ may be better.
We will add uninstall scripts later, right?

Sure, I have renamed to script folder.

I am not sure if uninstall scripts are needed in this scenario and I am open to discussion. This script is for testing only. ODBC users will be able to install and uninstall the ODBC driver using a msi executable, so they won't need to use the script to install or uninstall the driver. Developers can also manually delete the driver registry if needed. This script can also be run a second time to change the registry to point to a different ODBC dll location, so the driver registry usually doesn't need to be deleted

OK. Then how about using test\ directory?

Or ci\scripts\?

I have renamed to tests. I think tests directory would work better in this case compared to ci\scripts since this is the only script for testing.

raulcd · 2025-10-10T08:16:21Z

cpp/src/arrow/flight/sql/odbc/odbc_impl/address_info.h

 #include "arrow/flight/sql/odbc/odbc_impl/platform.h"

 #include <sys/types.h>
+#include <cstdint>


was this missing previously?

Yes, I think so. I recall it was causing build issues on workflows. The build works with or without this header on my local machine (Windows MSVC), but the CI builds can have problems without the header

alinaliBQ · 2025-10-15T00:12:17Z

I think all comments have been addressed, please let me know if you have more comments, thanks

alinaliBQ · 2025-10-15T23:56:12Z

I think the failed Python / AMD64 Windows 2022 Python 3.13 (pull_request) check is not due to the changes in this PR

lidavidm · 2025-10-16T00:34:53Z

(FWIW, Kou/Raul are probably busy getting the new release out)

raulcd

it seems you have pushed a change to the testing submodule reverting it back to the previous commit, can you update the submodule to point again to the latest commit?
Current commit should be:
apache/arrow-testing@9a02925

raulcd · 2025-10-17T09:15:20Z

Thanks a lot for the PR, CI seems to be fixed now :)

Enable ODBC build in Windows MSVC workflow Fix build issue Register ODBC driver ODBC driver registration is required for ODBC tests (enabled in a separate PR) Address Sutou's comments

alinaliBQ · 2025-10-17T18:56:44Z

it seems you have pushed a change to the testing submodule reverting it back to the previous commit, can you update the submodule to point again to the latest commit? Current commit should be: apache/arrow-testing@9a02925

Good catch, I have fixed it and reverted the testing submodule change. Thanks

kou

+1

github-actions bot added Component: C++ Component: FlightRPC awaiting review Awaiting review labels Oct 2, 2025

alinaliBQ commented Oct 2, 2025

View reviewed changes

github-actions bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Oct 2, 2025

kou changed the title ~~GH-46004: [C++][FlightRPC] Enable ODBC Build In C++ and Glib Workflows~~ GH-46004: [C++][FlightRPC] Enable ODBC Build In C++ and GLib Workflows Oct 3, 2025

alinaliBQ commented Oct 8, 2025

View reviewed changes

alinaliBQ force-pushed the gh-46004-build-odbc branch 2 times, most recently from c1d79fa to bb49757 Compare October 8, 2025 16:58

alinaliBQ changed the title ~~GH-46004: [C++][FlightRPC] Enable ODBC Build In C++ and GLib Workflows~~ GH-46004: [C++][FlightRPC] Enable ODBC Build In C++ Workflows Oct 8, 2025

alinaliBQ force-pushed the gh-46004-build-odbc branch 5 times, most recently from ec39f37 to 4d254df Compare October 9, 2025 19:49

alinaliBQ marked this pull request as ready for review October 9, 2025 20:22

alinaliBQ requested review from assignUser, jonkeane, kou, lidavidm and raulcd as code owners October 9, 2025 20:22

kou reviewed Oct 10, 2025

View reviewed changes

github-actions bot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels Oct 10, 2025

raulcd reviewed Oct 10, 2025

View reviewed changes

github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes awaiting change review Awaiting change review labels Oct 10, 2025

github-actions bot added the awaiting changes Awaiting changes label Oct 11, 2025

alinaliBQ force-pushed the gh-46004-build-odbc branch from ad772ef to 032cb79 Compare October 15, 2025 00:10

github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Oct 15, 2025

alinaliBQ requested review from kou and raulcd October 15, 2025 00:11

alinaliBQ force-pushed the gh-46004-build-odbc branch from 032cb79 to c880d47 Compare October 16, 2025 18:19

raulcd reviewed Oct 17, 2025

View reviewed changes

github-actions bot added awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels Oct 17, 2025

Build ODBC in C++ Windows Workflow

8d79a82

Enable ODBC build in Windows MSVC workflow Fix build issue Register ODBC driver ODBC driver registration is required for ODBC tests (enabled in a separate PR) Address Sutou's comments

alinaliBQ force-pushed the gh-46004-build-odbc branch from c880d47 to 8d79a82 Compare October 17, 2025 18:55

github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Oct 17, 2025

kou approved these changes Oct 17, 2025

View reviewed changes

github-actions bot added awaiting merge Awaiting merge and removed awaiting change review Awaiting change review labels Oct 17, 2025

	# ifdef NDEBUG
	# define ARROW_DFATAL ::arrow::util::ArrowLogLevel::ARROW_WARNING

	// CAUTION: DCHECK_OK() always evaluates its argument, but other DCHECK*() macros
	// only do so in debug mode.

	# define ARROW_DCHECK(condition) \
	while (false) ARROW_IGNORE_EXPR(condition); \
	while (false) ::arrow::util::detail::NullLog()
	# define ARROW_DCHECK_OK(s) \
	ARROW_IGNORE_EXPR(s); \
	while (false) ::arrow::util::detail::NullLog()
	# define ARROW_DCHECK_EQ(val1, val2) \
	while (false) ARROW_IGNORE_EXPR(val1); \
	while (false) ARROW_IGNORE_EXPR(val2); \
	while (false) ::arrow::util::detail::NullLog()
	# define ARROW_DCHECK_NE(val1, val2) \
	while (false) ARROW_IGNORE_EXPR(val1); \
	while (false) ARROW_IGNORE_EXPR(val2); \
	while (false) ::arrow::util::detail::NullLog()
	# define ARROW_DCHECK_LE(val1, val2) \
	while (false) ARROW_IGNORE_EXPR(val1); \
	while (false) ARROW_IGNORE_EXPR(val2); \
	while (false) ::arrow::util::detail::NullLog()
	# define ARROW_DCHECK_LT(val1, val2) \
	while (false) ARROW_IGNORE_EXPR(val1); \
	while (false) ARROW_IGNORE_EXPR(val2); \
	while (false) ::arrow::util::detail::NullLog()
	# define ARROW_DCHECK_GE(val1, val2) \
	while (false) ARROW_IGNORE_EXPR(val1); \
	while (false) ARROW_IGNORE_EXPR(val2); \
	while (false) ::arrow::util::detail::NullLog()
	# define ARROW_DCHECK_GT(val1, val2) \
	while (false) ARROW_IGNORE_EXPR(val1); \
	while (false) ARROW_IGNORE_EXPR(val2); \
	while (false) ::arrow::util::detail::NullLog()

GH-46004: [C++][FlightRPC] Enable ODBC Build In C++ Workflows #47689

Are you sure you want to change the base?

GH-46004: [C++][FlightRPC] Enable ODBC Build In C++ Workflows #47689

Conversation

alinaliBQ commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alinaliBQ Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alinaliBQ commented Oct 9, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alinaliBQ commented Oct 15, 2025

Uh oh!

alinaliBQ commented Oct 15, 2025

Uh oh!

lidavidm commented Oct 16, 2025

Uh oh!

raulcd left a comment

Choose a reason for hiding this comment

Uh oh!

raulcd commented Oct 17, 2025

Uh oh!

alinaliBQ commented Oct 17, 2025

Uh oh!

kou left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

alinaliBQ commented Oct 2, 2025 •

edited

Loading

alinaliBQ Oct 8, 2025 •

edited

Loading