Skip to content

GH-48312: [C++][FlightRPC] Standalone ODBC MSVC CI #48313

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

Merged
kou merged 7 commits into from
Dec 17, 2025
Merged

GH-48312: [C++][FlightRPC] Standalone ODBC MSVC CI #48313

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
af8a131
ODBC Standalone MSVC Build CI
alinaliBQ Dec 3, 2025
8dc692a
Address comments from Kou
alinaliBQ Dec 4, 2025
7841861
Attempt to move ODBC CI to `cpp_extra.yml`
alinaliBQ Dec 5, 2025
a25015a
Keep only `ARROW_FLIGHT_SQL_ODBC` flag on `cpp_odbc.yml`
alinaliBQ Dec 8, 2025
fed94f0
Address Kou's comments
alinaliBQ Dec 15, 2025
20d2e85
Move cpp_odbc to cpp_extra
alinaliBQ Dec 15, 2025
4ad7af1
Address Kou's comments Dec-16
alinaliBQ Dec 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 107 additions & 0 deletions .github/workflows/cpp_extra.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,12 +330,119 @@ jobs:
cd cpp/examples/minimal_build
../minimal_build.build/arrow-example

odbc:
needs: check-labels
name: ODBC
runs-on: windows-2022
if: >-
needs.check-labels.outputs.force == 'true' ||
contains(fromJSON(needs.check-labels.outputs.ci-extra-labels || '[]'), 'CI: Extra') ||
contains(fromJSON(needs.check-labels.outputs.ci-extra-labels || '[]'), 'CI: Extra: C++')
timeout-minutes: 240
env:
ARROW_BUILD_SHARED: ON
ARROW_BUILD_STATIC: OFF
ARROW_BUILD_TESTS: ON
ARROW_BUILD_TYPE: release
ARROW_DEPENDENCY_SOURCE: VCPKG
ARROW_FLIGHT_SQL_ODBC: ON
ARROW_SIMD_LEVEL: AVX2
CMAKE_CXX_STANDARD: "17"
CMAKE_GENERATOR: Ninja
CMAKE_INSTALL_PREFIX: /usr
VCPKG_BINARY_SOURCES: 'clear;nuget,GitHub,readwrite'
VCPKG_DEFAULT_TRIPLET: x64-windows
steps:
- name: Disable Crash Dialogs
run: |
reg add `
"HKCU\SOFTWARE\Microsoft\Windows\Windows Error Reporting" `
/v DontShowUI `
/t REG_DWORD `
/d 1 `
/f
- name: Checkout Arrow
uses: actions/checkout@v6
with:
fetch-depth: 0
submodules: recursive
- name: Download Timezone Database
shell: bash
run: ci/scripts/download_tz_database.sh
- name: Install cmake
shell: bash
run: |
ci/scripts/install_cmake.sh 4.1.2 /usr
- name: Install ccache
shell: bash
run: |
ci/scripts/install_ccache.sh 4.12.1 /usr
- name: Setup ccache
shell: bash
run: |
ci/scripts/ccache_setup.sh
- name: ccache info
id: ccache-info
shell: bash
run: |
echo "cache-dir=$(ccache --get-config cache_dir)" >> $GITHUB_OUTPUT
- name: Cache ccache
uses: actions/cache@v4
with:
path: ${{ steps.ccache-info.outputs.cache-dir }}
key: cpp-odbc-ccache-windows-x64-${{ hashFiles('cpp/**') }}
restore-keys: cpp-odbc-ccache-windows-x64-
- name: Checkout vcpkg
uses: actions/checkout@v6
with:
fetch-depth: 0
path: vcpkg
repository: microsoft/vcpkg
- name: Bootstrap vcpkg
run: |
vcpkg\bootstrap-vcpkg.bat
$VCPKG_ROOT = $(Resolve-Path -LiteralPath "vcpkg").ToString()
Write-Output ${VCPKG_ROOT} | `
Out-File -FilePath ${Env:GITHUB_PATH} -Encoding utf8 -Append
Write-Output "VCPKG_ROOT=${VCPKG_ROOT}" | `
Out-File -FilePath ${Env:GITHUB_ENV} -Encoding utf8 -Append
- name: Setup NuGet credentials for vcpkg caching
shell: bash
run: |
$(vcpkg fetch nuget | tail -n 1) \
sources add \
-source "https://nuget.pkg.github.com/$GITHUB_REPOSITORY_OWNER/index.json" \
-storepasswordincleartext \
-name "GitHub" \
-username "$GITHUB_REPOSITORY_OWNER" \
-password "${{ secrets.GITHUB_TOKEN }}"
$(vcpkg fetch nuget | tail -n 1) \
setapikey "${{ secrets.GITHUB_TOKEN }}" \
-source "https://nuget.pkg.github.com/$GITHUB_REPOSITORY_OWNER/index.json"
- name: Build
shell: cmd
run: |
set VCPKG_ROOT_KEEP=%VCPKG_ROOT%
call "C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Auxiliary\Build\vcvarsall.bat" x64
set VCPKG_ROOT=%VCPKG_ROOT_KEEP%
bash -c "ci/scripts/cpp_build.sh $(pwd) $(pwd)/build"
- 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%\arrow_flight_sql_odbc.dll
# GH-48270 TODO: Resolve segementation fault during Arrow library unload
# GH-48269 TODO: Enable Flight & Flight SQL testing in MSVC CI
# GH-48547 TODO: enable ODBC tests after GH-48270 and GH-48269 are resolved.

# GH-47787 TODO: Build ODBC installer

report-extra-cpp:
if: github.event_name == 'schedule' && always()
needs:
- docker
- jni-linux
- jni-macos
- msvc-arm64
- odbc
Comment thread
alinaliBQ marked this conversation as resolved.
uses: ./.github/workflows/report_ci.yml
secrets: inherit
3 changes: 2 additions & 1 deletion ci/scripts/cpp_test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,8 @@ if [ "${ARROW_USE_MESON:-OFF}" = "OFF" ] && \
;;
esac
if [ -n "${VCPKG_ROOT}" ] && [ -n "${VCPKG_DEFAULT_TRIPLET}" ]; then
CMAKE_PREFIX_PATH+=";${VCPKG_ROOT}/installed/${VCPKG_DEFAULT_TRIPLET}"
# Search vcpkg before <prefix>/lib/cmake.
CMAKE_PREFIX_PATH="${VCPKG_ROOT}/installed/${VCPKG_DEFAULT_TRIPLET};${CMAKE_PREFIX_PATH}"
fi
cmake \
-S "${source_dir}/examples/minimal_build" \
Expand Down
1 change: 1 addition & 0 deletions cpp/cmake_modules/SetupCxxFlags.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ if(WIN32)
#
# ARROW-2986: Without /EHsc we get C4530 warning
set(CXX_COMMON_FLAGS "/W3 /EHsc")
string(APPEND CMAKE_CXX_FLAGS " /EHsc")
Copy link
Copy Markdown
Member

@kou kou Dec 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this? CXX_COMMON_FLAGS will be appended later:

arrow/cpp/CMakeLists.txt

Line 526 in 8afd140

set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${CXX_COMMON_FLAGS}")

Copy link
Copy Markdown
Collaborator Author

@alinaliBQ alinaliBQ Dec 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found this line was needed when I was working on a separate build. Locally on MSVC Visual Studio I didn't need this line, but on CI using Ninja I get C4530 warning when building GTestAlt, which was treated as an error. Adding this line resolved the issue below at FindGTestAlt.cmake.

Issue Log:

C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.44.35207\include\__msvc_ostream.hpp(781): error C2220: the following warning is treated as an error
C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.44.35207\include\__msvc_ostream.hpp(781): warning C4530: C++ exception handler used, but unwind semantics are not enabled. Specify /EHsc

GitHub Action Run Log: https://github.com/apache/arrow/actions/runs/19112837562/job/54614206545

Copy link
Copy Markdown
Member

@kou kou Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. Could you try this?

diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt
index 7286616c4f..c9f026f926 100644
--- a/cpp/CMakeLists.txt
+++ b/cpp/CMakeLists.txt
@@ -520,8 +520,6 @@ endif()
 set(PARQUET_PC_REQUIRES "")
 set(PARQUET_PC_REQUIRES_PRIVATE "")
 
-include(ThirdpartyToolchain)
-
 # Add common flags
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${CXX_COMMON_FLAGS}")
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${ARROW_CXXFLAGS}")
@@ -536,6 +534,8 @@ string(REPLACE "-std=c++17" "" CMAKE_C_FLAGS ${CMAKE_C_FLAGS})
 # Add C++-only flags, like -std=c++17
 set(CMAKE_CXX_FLAGS "${CXX_ONLY_FLAGS} ${CMAKE_CXX_FLAGS}")
 
+include(ThirdpartyToolchain)
+
 # ASAN / TSAN / UBSAN
 if(ARROW_FUZZING)
   set(ARROW_USE_COVERAGE ON)

Copy link
Copy Markdown
Collaborator Author

@alinaliBQ alinaliBQ Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I have pushed the changes for cpp/CMakeLists.txt and removed the new line I added

endif()

# Disable C5105 (macro expansion producing 'defined' has undefined
Expand Down
10 changes: 3 additions & 7 deletions cpp/src/arrow/flight/sql/odbc/odbc_impl/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,13 +129,9 @@ if(WIN32)
system_dsn.h)
endif()

target_link_libraries(arrow_odbc_spi_impl PUBLIC arrow_flight_sql_shared
arrow_compute_shared Boost::locale)

# Link libraries on MINGW64 and macOS
if(MINGW OR APPLE)
target_link_libraries(arrow_odbc_spi_impl PUBLIC ${ODBCINST})
endif()
target_link_libraries(arrow_odbc_spi_impl
PUBLIC arrow_flight_sql_shared arrow_compute_shared Boost::locale
${ODBCINST})

set_target_properties(arrow_odbc_spi_impl
PROPERTIES ARCHIVE_OUTPUT_DIRECTORY
Expand Down
6 changes: 4 additions & 2 deletions cpp/src/arrow/flight/sql/odbc/odbc_impl/get_info_cache.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@

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

#include <sql.h>
#include <sqlext.h>
#include "arrow/array.h"
#include "arrow/array/array_nested.h"
#include "arrow/flight/sql/api.h"
Expand All @@ -32,6 +30,10 @@
#include "arrow/flight/sql/odbc/odbc_impl/scalar_function_reporter.h"
#include "arrow/flight/sql/odbc/odbc_impl/util.h"

// Include ODBC headers after arrow headers to avoid conflicts with sql_info_undef.h
#include <sql.h>
#include <sqlext.h>

// Aliases for entries in SqlInfoOptions::SqlInfo that are defined here
// due to causing compilation errors conflicting with ODBC definitions.
#define ARROW_SQL_IDENTIFIER_CASE 503
Expand Down
1 change: 1 addition & 0 deletions cpp/src/arrow/flight/sql/odbc/odbc_impl/odbc_connection.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include "arrow/flight/sql/odbc/odbc_impl/spi/statement.h"
#include "arrow/flight/sql/odbc/odbc_impl/util.h"

// Include ODBC headers after arrow headers to avoid conflicts with sql_info_undef.h
#include <odbcinst.h>
#include <sql.h>
#include <sqlext.h>
Expand Down
3 changes: 3 additions & 0 deletions cpp/src/arrow/flight/sql/odbc/tests/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,6 @@ add_arrow_test(flight_sql_odbc_test
${ODBCINST}
${SQLite3_LIBRARIES}
arrow_odbc_spi_impl)

# Disable unity build due to sqlite_sql_info.cc conflict with sql.h and sqlext.h headers.
set_target_properties(arrow-flight-sql-odbc-test PROPERTIES UNITY_BUILD OFF)
2 changes: 2 additions & 0 deletions cpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
// specific language governing permissions and limitations
// under the License.

#pragma once

#include "arrow/testing/gtest_util.h"
#include "arrow/util/io_util.h"
#include "arrow/util/utf8.h"
Expand Down
Loading