From 4788fd9a7f12ed26b5619b37e1561b44c5351c9b Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Fri, 17 Oct 2025 11:53:43 -0700 Subject: [PATCH] Build ODBC in C++ Windows Workflow 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 --- .github/workflows/cpp.yml | 4 ++ ci/scripts/cpp_build.sh | 2 + cpp/cmake_modules/DefineOptions.cmake | 2 +- cpp/cmake_modules/ThirdpartyToolchain.cmake | 2 +- .../accessors/string_array_accessor.cc | 8 +-- .../accessors/string_array_accessor.h | 2 +- .../accessors/string_array_accessor_test.cc | 1 - .../flight/sql/odbc/odbc_impl/address_info.h | 1 + .../odbc/odbc_impl/flight_sql_connection.cc | 2 +- .../flight_sql_statement_get_columns.h | 2 + .../flight_sql_statement_get_type_info.h | 2 + .../sql/odbc/odbc_impl/system_trust_store.cc | 2 +- .../sql/odbc/odbc_impl/system_trust_store.h | 6 ++- .../sql/odbc/odbc_impl/ui/custom_window.cc | 6 ++- .../flight/sql/odbc/tests/install_odbc.cmd | 53 +++++++++++++++++++ 15 files changed, 82 insertions(+), 13 deletions(-) create mode 100644 cpp/src/arrow/flight/sql/odbc/tests/install_odbc.cmd diff --git a/.github/workflows/cpp.yml b/.github/workflows/cpp.yml index db3abd456cd..0ed16bd6703 100644 --- a/.github/workflows/cpp.yml +++ b/.github/workflows/cpp.yml @@ -389,6 +389,10 @@ jobs: PIPX_BASE_PYTHON: ${{ steps.python-install.outputs.python-path }} run: | ci/scripts/install_gcs_testbench.sh default + - name: Register Flight SQL ODBC Driver + shell: cmd + run: | + call "cpp\src\arrow\flight\sql\odbc\tests\install_odbc.cmd" ${{ github.workspace }}\build\cpp\%ARROW_BUILD_TYPE%\libarrow_flight_sql_odbc.dll - name: Test shell: msys2 {0} run: | diff --git a/ci/scripts/cpp_build.sh b/ci/scripts/cpp_build.sh index 79fc79cc724..eb4291bafbd 100755 --- a/ci/scripts/cpp_build.sh +++ b/ci/scripts/cpp_build.sh @@ -64,6 +64,7 @@ if [ "${ARROW_ENABLE_THREADING:-ON}" = "OFF" ]; then ARROW_AZURE=OFF ARROW_FLIGHT=OFF ARROW_FLIGHT_SQL=OFF + ARROW_FLIGHT_SQL_ODBC=OFF ARROW_GCS=OFF ARROW_JEMALLOC=OFF ARROW_MIMALLOC=OFF @@ -212,6 +213,7 @@ else -DARROW_FILESYSTEM=${ARROW_FILESYSTEM:-ON} \ -DARROW_FLIGHT=${ARROW_FLIGHT:-OFF} \ -DARROW_FLIGHT_SQL=${ARROW_FLIGHT_SQL:-OFF} \ + -DARROW_FLIGHT_SQL_ODBC=${ARROW_FLIGHT_SQL_ODBC:-OFF} \ -DARROW_FUZZING=${ARROW_FUZZING:-OFF} \ -DARROW_GANDIVA_PC_CXX_FLAGS=${ARROW_GANDIVA_PC_CXX_FLAGS:-} \ -DARROW_GANDIVA=${ARROW_GANDIVA:-OFF} \ diff --git a/cpp/cmake_modules/DefineOptions.cmake b/cpp/cmake_modules/DefineOptions.cmake index dfd99e4c7a4..0f6674c7143 100644 --- a/cpp/cmake_modules/DefineOptions.cmake +++ b/cpp/cmake_modules/DefineOptions.cmake @@ -108,7 +108,7 @@ endmacro() macro(resolve_option_dependencies) # Arrow Flight SQL ODBC is available only for Windows for now. - if(NOT MSVC_TOOLCHAIN) + if(NOT WIN32) set(ARROW_FLIGHT_SQL_ODBC OFF) endif() if(MSVC_TOOLCHAIN) diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index 17158ae331d..232ee64d9f8 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -1274,7 +1274,7 @@ if(ARROW_USE_BOOST) endif() if(ARROW_BOOST_REQUIRE_LIBRARY) set(ARROW_BOOST_COMPONENTS filesystem system) - if(ARROW_FLIGHT_SQL_ODBC AND MSVC) + if(ARROW_FLIGHT_SQL_ODBC) list(APPEND ARROW_BOOST_COMPONENTS locale) endif() if(ARROW_ENABLE_THREADING) diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/string_array_accessor.cc b/cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/string_array_accessor.cc index 340b68620be..69b3b304945 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/string_array_accessor.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/string_array_accessor.cc @@ -24,7 +24,7 @@ namespace arrow::flight::sql::odbc { namespace { -#if defined _WIN32 || defined _WIN64 +#if defined _WIN32 std::string Utf8ToCLocale(const char* utf8_str, int len) { thread_local boost::locale::generator g; g.locale_cache_enabled(true); @@ -36,7 +36,7 @@ std::string Utf8ToCLocale(const char* utf8_str, int len) { template inline RowStatus MoveSingleCellToCharBuffer( std::vector& buffer, int64_t& last_retrieved_arrow_row, -#if defined _WIN32 || defined _WIN64 +#if defined _WIN32 std::string& clocale_str, #endif ColumnBinding* binding, StringArray* array, int64_t arrow_row, int64_t i, @@ -57,7 +57,7 @@ inline RowStatus MoveSingleCellToCharBuffer( value = buffer.data(); size_in_bytes = buffer.size(); } else { -#if defined _WIN32 || defined _WIN64 +#if defined _WIN32 // Convert to C locale string if (last_retrieved_arrow_row != arrow_row) { clocale_str = Utf8ToCLocale(raw_value, raw_value_length); @@ -124,7 +124,7 @@ RowStatus StringArrayFlightSqlAccessor::MoveSingleCellIm ColumnBinding* binding, int64_t arrow_row, int64_t i, int64_t& value_offset, bool update_value_offset, Diagnostics& diagnostics) { return MoveSingleCellToCharBuffer(buffer_, last_arrow_row_, -#if defined _WIN32 || defined _WIN64 +#if defined _WIN32 clocale_str_, #endif binding, this->GetArray(), arrow_row, i, diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/string_array_accessor.h b/cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/string_array_accessor.h index db68084f408..60a8e37f577 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/string_array_accessor.h +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/string_array_accessor.h @@ -41,7 +41,7 @@ class StringArrayFlightSqlAccessor private: std::vector buffer_; -#if defined _WIN32 || defined _WIN64 +#if defined _WIN32 std::string clocale_str_; #endif int64_t last_arrow_row_; diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/string_array_accessor_test.cc b/cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/string_array_accessor_test.cc index f0d8f898ba9..eb7a9c88b3b 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/string_array_accessor_test.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/string_array_accessor_test.cc @@ -126,7 +126,6 @@ TEST(StringArrayAccessor, Test_CDataType_WCHAR_Truncation) { ColumnBinding binding(CDataType_WCHAR, 0, 0, buffer.data(), max_str_len, str_len_buffer.data()); - std::basic_stringstream ss; int64_t value_offset = 0; // Construct the whole string by concatenating smaller chunks from diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/address_info.h b/cpp/src/arrow/flight/sql/odbc/odbc_impl/address_info.h index 6ba9ca23b80..c127c0f4a29 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/address_info.h +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/address_info.h @@ -22,6 +22,7 @@ #include "arrow/flight/sql/odbc/odbc_impl/platform.h" #include +#include #if !_WIN32 # include #endif diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_connection.cc b/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_connection.cc index b4f9c69b39b..479a72f3fea 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_connection.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_connection.cc @@ -212,7 +212,7 @@ boost::optional FlightSqlConnection::GetStringColumnLength( } bool FlightSqlConnection::GetUseWideChar(const ConnPropertyMap& conn_property_map) { -#if defined _WIN32 || defined _WIN64 +#if defined _WIN32 // Windows should use wide chars by default bool default_value = true; #else diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_statement_get_columns.h b/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_statement_get_columns.h index 39f0cce1eac..b764fe76e3a 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_statement_get_columns.h +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_statement_get_columns.h @@ -15,6 +15,8 @@ // specific language governing permissions and limitations // under the License. +#pragma once + #include #include "arrow/array/builder_binary.h" #include "arrow/array/builder_primitive.h" diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_statement_get_type_info.h b/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_statement_get_type_info.h index 7741d71bd4b..3f47d88bbd3 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_statement_get_type_info.h +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_statement_get_type_info.h @@ -15,6 +15,8 @@ // specific language governing permissions and limitations // under the License. +#pragma once + #include #include "arrow/array/builder_binary.h" #include "arrow/array/builder_primitive.h" diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/system_trust_store.cc b/cpp/src/arrow/flight/sql/odbc/odbc_impl/system_trust_store.cc index d7d2e72b4b5..14302ed5a62 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/system_trust_store.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/system_trust_store.cc @@ -20,7 +20,7 @@ #include "arrow/flight/sql/odbc/odbc_impl/system_trust_store.h" -#if defined _WIN32 || defined _WIN64 +#if defined _WIN32 namespace arrow::flight::sql::odbc { bool SystemTrustStore::HasNext() { diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/system_trust_store.h b/cpp/src/arrow/flight/sql/odbc/odbc_impl/system_trust_store.h index 94ad924aaa3..499eba1d1f8 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/system_trust_store.h +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/system_trust_store.h @@ -17,13 +17,17 @@ #pragma once -#if defined _WIN32 || defined _WIN64 +#if defined _WIN32 # include # include # include + +// prsht.h needs to be included before cryptuiapi.h to avoid build conflict +# include + # include # include diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/ui/custom_window.cc b/cpp/src/arrow/flight/sql/odbc/odbc_impl/ui/custom_window.cc index 9aded323d0d..179303b68e3 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/ui/custom_window.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/ui/custom_window.cc @@ -19,6 +19,8 @@ // before Windowsx.h and commctrl.h #include "arrow/flight/sql/odbc/odbc_impl/platform.h" +#include "arrow/util/logging.h" + #include #include @@ -52,7 +54,7 @@ LRESULT CALLBACK CustomWindow::WndProc(HWND hwnd, UINT msg, WPARAM wparam, switch (msg) { case WM_NCCREATE: { - _ASSERT(lparam != NULL); + ARROW_DCHECK_NE(lparam, NULL); CREATESTRUCT* create_struct = reinterpret_cast(lparam); @@ -64,7 +66,7 @@ LRESULT CALLBACK CustomWindow::WndProc(HWND hwnd, UINT msg, WPARAM wparam, } case WM_CREATE: { - _ASSERT(window != NULL); + ARROW_DCHECK_NE(window, NULL); window->SetHandle(hwnd); diff --git a/cpp/src/arrow/flight/sql/odbc/tests/install_odbc.cmd b/cpp/src/arrow/flight/sql/odbc/tests/install_odbc.cmd new file mode 100644 index 00000000000..dc09bf781d6 --- /dev/null +++ b/cpp/src/arrow/flight/sql/odbc/tests/install_odbc.cmd @@ -0,0 +1,53 @@ +@REM Licensed to the Apache Software Foundation (ASF) under one +@REM or more contributor license agreements. See the NOTICE file +@REM distributed with this work for additional information +@REM regarding copyright ownership. The ASF licenses this file +@REM to you under the Apache License, Version 2.0 (the +@REM "License"); you may not use this file except in compliance +@REM with the License. You may obtain a copy of the License at +@REM +@REM http://www.apache.org/licenses/LICENSE-2.0 +@REM +@REM Unless required by applicable law or agreed to in writing, +@REM software distributed under the License is distributed on an +@REM "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +@REM KIND, either express or implied. See the License for the +@REM specific language governing permissions and limitations +@REM under the License. + +@echo off + +set ODBC_64BIT=%1 + +@REM enable delayed variable expansion to make environment variables enclosed with "!" to be evaluated +@REM when the command is executed instead of when the command is parsed +setlocal enableextensions enabledelayedexpansion + +if [%ODBC_64BIT%] == [] ( + echo error: 64-bit driver is not specified. Call format: install_odbc abs_path_to_64_bit_driver + pause + exit /b 1 +) + +if exist %ODBC_64BIT% ( + for %%i IN (%ODBC_64BIT%) DO IF EXIST %%~si\NUL ( + echo warning: The path you have specified seems to be a directory. Note that you have to specify path to driver file itself instead. + ) + echo Installing 64-bit driver: %ODBC_64BIT% + reg add "HKEY_LOCAL_MACHINE\SOFTWARE\ODBC\ODBCINST.INI\Apache Arrow Flight SQL ODBC Driver" /v DriverODBCVer /t REG_SZ /d "03.80" /f + reg add "HKEY_LOCAL_MACHINE\SOFTWARE\ODBC\ODBCINST.INI\Apache Arrow Flight SQL ODBC Driver" /v UsageCount /t REG_DWORD /d 00000001 /f + reg add "HKEY_LOCAL_MACHINE\SOFTWARE\ODBC\ODBCINST.INI\Apache Arrow Flight SQL ODBC Driver" /v Driver /t REG_SZ /d %ODBC_64BIT% /f + reg add "HKEY_LOCAL_MACHINE\SOFTWARE\ODBC\ODBCINST.INI\Apache Arrow Flight SQL ODBC Driver" /v Setup /t REG_SZ /d %ODBC_64BIT% /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 ( + echo Error occurred while registering 64-bit driver. Exiting. + echo ERRORLEVEL: !ERRORLEVEL! + exit !ERRORLEVEL! + ) +) else ( + echo 64-bit driver can not be found: %ODBC_64BIT% + echo Call format: install_odbc abs_path_to_64_bit_driver + pause + exit /b 1 +)