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

Enable ODBC build in Windows MSVC workflow

ODBC driver registration is required for ODBC tests (enabled in a separate PR)

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/script/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

kou · 2025-10-10T00:39:43Z

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


 #pragma once

 #if defined _WIN32 || defined _WIN64


Do we need || defined _WIN64 here?

https://learn.microsoft.com/en-us/cpp/preprocessor/predefined-macros?view=msvc-170

_WIN32 Defined as 1 when the compilation target is 32-bit ARM, 64-bit ARM, x86, x64, or ARM64EC. Otherwise, undefined.

I think it can be removed. Fixed

kou · 2025-10-10T00:43:51Z

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

+# Compile entry_points.cc before odbc_api.cc due to conflict from sql.h and flight/types.h
+set(ARROW_FLIGHT_SQL_ODBC_SRCS odbc_api.cc entry_points.cc)


Could you share build log URL without this?

We may just need to disable unity build like https://github.com/apache/arrow/pull/47627/files#diff-2a4796eff3f25e1b0fd6a0db28bdea1efbc3ec69457705b1069cf70ad5355624R192-R204 does.

Our team discovered the build failure a while ago so I don't have the error log. I have reverted this change. If the build fails because of it then I can send the log here

.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\?

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

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 from bb49757 to 7d0c9ad Compare October 8, 2025 17:38

Build ODBC in C++ Windows Workflow

5c08a8a

Enable ODBC build in Windows MSVC workflow

alinaliBQ force-pushed the gh-46004-build-odbc branch 2 times, most recently from f97a86d to 078841c Compare October 9, 2025 17:25

Fix build issue

83df835

alinaliBQ force-pushed the gh-46004-build-odbc branch from 078841c to 83df835 Compare October 9, 2025 19:17

Register ODBC driver

4d254df

ODBC driver registration is required for ODBC tests (enabled in a separate PR)

alinaliBQ force-pushed the gh-46004-build-odbc branch 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

Address Sutou's comments

20545e8

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

Fix build and lint issues

ad772ef

github-actions bot added awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels Oct 11, 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()

		# Compile entry_points.cc before odbc_api.cc due to conflict from sql.h and flight/types.h
		set(ARROW_FLIGHT_SQL_ODBC_SRCS odbc_api.cc entry_points.cc)

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!

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

alinaliBQ commented Oct 2, 2025 •

edited

Loading

alinaliBQ Oct 8, 2025 •

edited

Loading