From 778cbd2faea3f1011738a45b0617fe2cbaeb4cfa Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Tue, 4 Nov 2025 16:22:48 -0800 Subject: [PATCH 01/35] Enable ODBC build on MSVC CI --- .github/workflows/cpp_windows.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/cpp_windows.yml b/.github/workflows/cpp_windows.yml index cc48f277104..182280844e8 100644 --- a/.github/workflows/cpp_windows.yml +++ b/.github/workflows/cpp_windows.yml @@ -45,6 +45,7 @@ jobs: ARROW_BUILD_TESTS: ON ARROW_DATASET: ON ARROW_FLIGHT: OFF + ARROW_FLIGHT_SQL_ODBC: ON ARROW_HDFS: ON ARROW_HOME: /usr ARROW_JEMALLOC: OFF @@ -115,6 +116,10 @@ jobs: run: | call "C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Auxiliary\Build\vcvarsall.bat" ${{ inputs.arch }} 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%\libarrow_flight_sql_odbc.dll - name: Test shell: cmd run: | From eb2a6f2a8f856cbdbf6468ea2f3d67c1df045d0d Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Tue, 4 Nov 2025 16:35:56 -0800 Subject: [PATCH 02/35] Enable Flight & Flight SQL for ODBC --- .github/workflows/cpp_windows.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/cpp_windows.yml b/.github/workflows/cpp_windows.yml index 182280844e8..4a877642e09 100644 --- a/.github/workflows/cpp_windows.yml +++ b/.github/workflows/cpp_windows.yml @@ -44,7 +44,8 @@ jobs: ARROW_BUILD_STATIC: OFF ARROW_BUILD_TESTS: ON ARROW_DATASET: ON - ARROW_FLIGHT: OFF + ARROW_FLIGHT: ON + ARROW_FLIGHT_SQL: ON ARROW_FLIGHT_SQL_ODBC: ON ARROW_HDFS: ON ARROW_HOME: /usr From f9b64c8b31203900aafc9d792adeec9be022a61f Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Tue, 4 Nov 2025 16:47:04 -0800 Subject: [PATCH 03/35] Remove `ARROW_BOOST_USE_SHARED` Having static vs. shared issue --- .github/workflows/cpp_windows.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/cpp_windows.yml b/.github/workflows/cpp_windows.yml index 4a877642e09..7ea110986cf 100644 --- a/.github/workflows/cpp_windows.yml +++ b/.github/workflows/cpp_windows.yml @@ -38,7 +38,6 @@ jobs: runs-on: ${{ inputs.os }} timeout-minutes: 60 env: - ARROW_BOOST_USE_SHARED: OFF ARROW_BUILD_BENCHMARKS: ON ARROW_BUILD_SHARED: ON ARROW_BUILD_STATIC: OFF From 6c6faf0e5c83fc7242478a9efc9ce18d24b42eb2 Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Tue, 4 Nov 2025 16:52:29 -0800 Subject: [PATCH 04/35] `ARROW_BUILD_BENCHMARKS` prevents Flight from being built --- .github/workflows/cpp_windows.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/cpp_windows.yml b/.github/workflows/cpp_windows.yml index 7ea110986cf..17601cc2009 100644 --- a/.github/workflows/cpp_windows.yml +++ b/.github/workflows/cpp_windows.yml @@ -38,7 +38,6 @@ jobs: runs-on: ${{ inputs.os }} timeout-minutes: 60 env: - ARROW_BUILD_BENCHMARKS: ON ARROW_BUILD_SHARED: ON ARROW_BUILD_STATIC: OFF ARROW_BUILD_TESTS: ON From e81260fb2687a466249c51cd5ab77a165cb8f28e Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Tue, 4 Nov 2025 17:13:57 -0800 Subject: [PATCH 05/35] Enable static build on MSVC --- .github/workflows/cpp_windows.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/cpp_windows.yml b/.github/workflows/cpp_windows.yml index 17601cc2009..4ba08d2efab 100644 --- a/.github/workflows/cpp_windows.yml +++ b/.github/workflows/cpp_windows.yml @@ -39,7 +39,7 @@ jobs: timeout-minutes: 60 env: ARROW_BUILD_SHARED: ON - ARROW_BUILD_STATIC: OFF + ARROW_BUILD_STATIC: ON ARROW_BUILD_TESTS: ON ARROW_DATASET: ON ARROW_FLIGHT: ON From adc4cceac83c34334784c89d17b44ab5581b231e Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Wed, 5 Nov 2025 09:16:57 -0800 Subject: [PATCH 06/35] Set ARROW_DEPENDENCY_SOURCE to VCPKG To make it easier to manage dependencies --- .github/workflows/cpp_windows.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/cpp_windows.yml b/.github/workflows/cpp_windows.yml index 4ba08d2efab..2b9c308eb7c 100644 --- a/.github/workflows/cpp_windows.yml +++ b/.github/workflows/cpp_windows.yml @@ -42,6 +42,7 @@ jobs: ARROW_BUILD_STATIC: ON ARROW_BUILD_TESTS: ON ARROW_DATASET: ON + ARROW_DEPENDENCY_SOURCE: VCPKG ARROW_FLIGHT: ON ARROW_FLIGHT_SQL: ON ARROW_FLIGHT_SQL_ODBC: ON From d67f3fe61579264bbf326bf826567f9d6e03b54d Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Wed, 5 Nov 2025 09:20:54 -0800 Subject: [PATCH 07/35] Specify VCPKG_TARGET_TRIPLET as x64 windows --- .github/workflows/cpp_windows.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/cpp_windows.yml b/.github/workflows/cpp_windows.yml index 2b9c308eb7c..e8ca2ce33fb 100644 --- a/.github/workflows/cpp_windows.yml +++ b/.github/workflows/cpp_windows.yml @@ -68,6 +68,7 @@ jobs: CMAKE_GENERATOR: Ninja CMAKE_INSTALL_PREFIX: /usr CMAKE_UNITY_BUILD: ON + VCPKG_TARGET_TRIPLET: x64-windows steps: - name: Disable Crash Dialogs run: | From feede59fe49a205ada7e3bf536b18288720ee605 Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Wed, 5 Nov 2025 09:30:51 -0800 Subject: [PATCH 08/35] Specify `VCPKG_BINARY_SOURCES` and `VCPKG_DEFAULT_TRIPLET` --- .github/workflows/cpp_windows.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/cpp_windows.yml b/.github/workflows/cpp_windows.yml index e8ca2ce33fb..8f81b38a1b6 100644 --- a/.github/workflows/cpp_windows.yml +++ b/.github/workflows/cpp_windows.yml @@ -68,7 +68,8 @@ jobs: CMAKE_GENERATOR: Ninja CMAKE_INSTALL_PREFIX: /usr CMAKE_UNITY_BUILD: ON - VCPKG_TARGET_TRIPLET: x64-windows + VCPKG_BINARY_SOURCES: 'clear;nuget,GitHub,readwrite' + VCPKG_DEFAULT_TRIPLET: x64-windows steps: - name: Disable Crash Dialogs run: | From 20183874617150878c6b74a3469135f8017124b0 Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Wed, 5 Nov 2025 10:48:47 -0800 Subject: [PATCH 09/35] Empty commit to trigger CI From 79257b4711d0160cb44cecf71040727b32fe00b3 Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Wed, 5 Nov 2025 10:50:47 -0800 Subject: [PATCH 10/35] Extend run-time to 2hr windows-mingw also has timeout of 2hr --- .github/workflows/cpp_windows.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/cpp_windows.yml b/.github/workflows/cpp_windows.yml index 8f81b38a1b6..546de190796 100644 --- a/.github/workflows/cpp_windows.yml +++ b/.github/workflows/cpp_windows.yml @@ -36,7 +36,7 @@ on: jobs: windows: runs-on: ${{ inputs.os }} - timeout-minutes: 60 + timeout-minutes: 120 env: ARROW_BUILD_SHARED: ON ARROW_BUILD_STATIC: ON From 3fd52ad2e90fe531acf9f4995229ffda27a0b49b Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Wed, 5 Nov 2025 14:05:25 -0800 Subject: [PATCH 11/35] Remove `CMAKE_CXX_STANDARD` 17 * reason: gtest can't be used with C++17. Arrow project doesn't support C++ 17 yet. --- .github/workflows/cpp_windows.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/cpp_windows.yml b/.github/workflows/cpp_windows.yml index 546de190796..23be3c19713 100644 --- a/.github/workflows/cpp_windows.yml +++ b/.github/workflows/cpp_windows.yml @@ -64,7 +64,6 @@ jobs: ARROW_WITH_ZLIB: ON ARROW_WITH_ZSTD: ON BOOST_SOURCE: BUNDLED - CMAKE_CXX_STANDARD: "17" CMAKE_GENERATOR: Ninja CMAKE_INSTALL_PREFIX: /usr CMAKE_UNITY_BUILD: ON From f1493e8308bdf90058e542826b2ba513e040803c Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Wed, 5 Nov 2025 14:57:03 -0800 Subject: [PATCH 12/35] Add `/EHsc` flag * Add back `CMAKE_CXX_STANDARD: "17"` --- .github/workflows/cpp_windows.yml | 1 + cpp/cmake_modules/SetupCxxFlags.cmake | 1 + 2 files changed, 2 insertions(+) diff --git a/.github/workflows/cpp_windows.yml b/.github/workflows/cpp_windows.yml index 23be3c19713..546de190796 100644 --- a/.github/workflows/cpp_windows.yml +++ b/.github/workflows/cpp_windows.yml @@ -64,6 +64,7 @@ jobs: ARROW_WITH_ZLIB: ON ARROW_WITH_ZSTD: ON BOOST_SOURCE: BUNDLED + CMAKE_CXX_STANDARD: "17" CMAKE_GENERATOR: Ninja CMAKE_INSTALL_PREFIX: /usr CMAKE_UNITY_BUILD: ON diff --git a/cpp/cmake_modules/SetupCxxFlags.cmake b/cpp/cmake_modules/SetupCxxFlags.cmake index 3c172aebdf8..e0d1fa0dcca 100644 --- a/cpp/cmake_modules/SetupCxxFlags.cmake +++ b/cpp/cmake_modules/SetupCxxFlags.cmake @@ -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") endif() # Disable C5105 (macro expansion producing 'defined' has undefined From 99198bf293ae8e201c02176fd8e1cd9b76c6ab2a Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Thu, 6 Nov 2025 10:36:32 -0800 Subject: [PATCH 13/35] Add VCPKG set up Borrowed from the Glib workflow --- .github/workflows/cpp_windows.yml | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/.github/workflows/cpp_windows.yml b/.github/workflows/cpp_windows.yml index 546de190796..755b22f9fe2 100644 --- a/.github/workflows/cpp_windows.yml +++ b/.github/workflows/cpp_windows.yml @@ -113,10 +113,39 @@ jobs: path: ${{ steps.ccache-info.outputs.cache-dir }} key: cpp-ccache-windows-${{ inputs.arch }}-${{ hashFiles('cpp/**') }} restore-keys: cpp-ccache-windows-${{ inputs.arch }}- + - name: Checkout vcpkg + uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + 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" ${{ inputs.arch }} + set VCPKG_ROOT=%VCPKG_ROOT_KEEP% bash -c "ci/scripts/cpp_build.sh $(pwd) $(pwd)/build" - name: Register Flight SQL ODBC Driver shell: cmd From b6adbfd80246d0cb7c163ad9da8775b4c6cbbf07 Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Thu, 6 Nov 2025 13:31:18 -0800 Subject: [PATCH 14/35] Set ARROW_BOOST_USE_SHARED to OFF `ARROW_BOOST_USE_SHARED` is restored to `OFF`, which according to Windows doc should help with the debug build now. Retore value of `ARROW_BUILD_BENCHMARKS`, though noting it is set to `OFF` in GLib MSVC workflow. --- .github/workflows/cpp_windows.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/cpp_windows.yml b/.github/workflows/cpp_windows.yml index 755b22f9fe2..fa53c051047 100644 --- a/.github/workflows/cpp_windows.yml +++ b/.github/workflows/cpp_windows.yml @@ -38,6 +38,8 @@ jobs: runs-on: ${{ inputs.os }} timeout-minutes: 120 env: + ARROW_BOOST_USE_SHARED: OFF + ARROW_BUILD_BENCHMARKS: ON ARROW_BUILD_SHARED: ON ARROW_BUILD_STATIC: ON ARROW_BUILD_TESTS: ON From e3e38627150befb329d85044aca31c385a7225f3 Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Thu, 6 Nov 2025 16:39:01 -0800 Subject: [PATCH 15/35] Change to release build Getting ``` orc.lib(Exceptions.cc.obj) : error LNK2038: mismatch detected for '_ITERATOR_DEBUG_LEVEL': value '0' doesn't match value '2' in unity_2_cxx.cxx.obj orc.lib(Exceptions.cc.obj) : error LNK2038: mismatch detected for 'RuntimeLibrary': value 'MD_DynamicRelease' doesn't match value 'MDd_DynamicDebug' in unity_2_cxx.cxx.obj ``` when building with debug and presumably ARROW_ORC --- .github/workflows/cpp_windows.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/cpp_windows.yml b/.github/workflows/cpp_windows.yml index fa53c051047..df98ea7c29b 100644 --- a/.github/workflows/cpp_windows.yml +++ b/.github/workflows/cpp_windows.yml @@ -43,6 +43,7 @@ jobs: ARROW_BUILD_SHARED: ON ARROW_BUILD_STATIC: ON ARROW_BUILD_TESTS: ON + ARROW_BUILD_TYPE: release ARROW_DATASET: ON ARROW_DEPENDENCY_SOURCE: VCPKG ARROW_FLIGHT: ON From 36ea943ba74ee7e37bc8d5e1e831b16b30a1048f Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Fri, 7 Nov 2025 09:59:10 -0800 Subject: [PATCH 16/35] Remove `ARROW_BOOST_USE_SHARED = OFF` Since we have a release build, can enable boost as shared. With `ARROW_BOOST_USE_SHARED = OFF`, I am getting error ``` D:\a\arrow\arrow\build\cpp\vcpkg_installed\x64-windows\include\boost/filesystem/config.hpp(96): fatal error C1189: #error: Must not define both BOOST_FILESYSTEM_DYN_LINK and BOOST_FILESYSTEM_STATIC_LINK ``` Also increased time limit, since 2 hours don't seem to be enough to finish the vcpkg build --- .github/workflows/cpp_windows.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/cpp_windows.yml b/.github/workflows/cpp_windows.yml index df98ea7c29b..08a62b6f3a8 100644 --- a/.github/workflows/cpp_windows.yml +++ b/.github/workflows/cpp_windows.yml @@ -36,9 +36,8 @@ on: jobs: windows: runs-on: ${{ inputs.os }} - timeout-minutes: 120 + timeout-minutes: 240 env: - ARROW_BOOST_USE_SHARED: OFF ARROW_BUILD_BENCHMARKS: ON ARROW_BUILD_SHARED: ON ARROW_BUILD_STATIC: ON From 5b83635d6d295d84934526dae89867ac77922bc3 Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Fri, 7 Nov 2025 12:29:23 -0800 Subject: [PATCH 17/35] Remove `BOOST_SOURCE=BUNDLED` to use vcpkg's dynamic link to boost Since we need to use link to boost dynamically, we need to remove the bundled boost library flag. Add debug messages. --- .github/workflows/cpp_windows.yml | 1 - cpp/cmake_modules/ThirdpartyToolchain.cmake | 5 +++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/cpp_windows.yml b/.github/workflows/cpp_windows.yml index 08a62b6f3a8..cd8ad6af271 100644 --- a/.github/workflows/cpp_windows.yml +++ b/.github/workflows/cpp_windows.yml @@ -65,7 +65,6 @@ jobs: ARROW_WITH_SNAPPY: ON ARROW_WITH_ZLIB: ON ARROW_WITH_ZSTD: ON - BOOST_SOURCE: BUNDLED CMAKE_CXX_STANDARD: "17" CMAKE_GENERATOR: Ninja CMAKE_INSTALL_PREFIX: /usr diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index 1724c0d3a3d..7d87b639b98 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -1269,14 +1269,19 @@ else() endif() if(ARROW_USE_BOOST) + message(status "-AL- ARROW_BOOST_USE_SHARED: ${ARROW_BOOST_USE_SHARED}") if(ARROW_BOOST_USE_SHARED) # Find shared Boost libraries. set(Boost_USE_STATIC_LIBS OFF) set(BUILD_SHARED_LIBS_KEEP ${BUILD_SHARED_LIBS}) set(BUILD_SHARED_LIBS ON) + message(status "-AL- ARROW_BOOST_USE_SHARED TEMP - Boost_USE_STATIC_LIBS: ${Boost_USE_STATIC_LIBS}" + ) else() # Find static boost headers and libs set(Boost_USE_STATIC_LIBS ON) + message(status "-AL- NOT ARROW_BOOST_USE_SHARED TEMP - Boost_USE_STATIC_LIBS: ${Boost_USE_STATIC_LIBS}" + ) endif() if(ARROW_BOOST_REQUIRE_LIBRARY) set(ARROW_BOOST_COMPONENTS filesystem) From 6961176e2729f3e6ff2726bfcf01f8ae1c19d668 Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" <96995091+alinaliBQ@users.noreply.github.com> Date: Thu, 2 Oct 2025 15:47:04 -0700 Subject: [PATCH 18/35] [GH-48084] Replace boost::optional with std::optional Addresses comment https://github.com/apache/arrow/pull/40939#discussion_r2099118497 Replace boost::optional with std::optional in ODBC codebase https://github.com/apache/arrow/issues/48084 Fix lint --- .../sql/odbc/odbc_impl/blocking_queue.h | 6 +++--- .../odbc/odbc_impl/flight_sql_auth_method.cc | 3 ++- .../odbc/odbc_impl/flight_sql_connection.cc | 19 +++++++++++-------- .../odbc/odbc_impl/flight_sql_connection.h | 6 +++--- .../odbc_impl/flight_sql_connection_test.cc | 10 ++++++---- .../odbc/odbc_impl/flight_sql_statement.cc | 14 +++++++++----- .../sql/odbc/odbc_impl/flight_sql_statement.h | 6 ++++-- .../flight_sql_stream_chunk_buffer.cc | 10 +++++++++- .../sql/odbc/odbc_impl/odbc_connection.cc | 3 ++- .../sql/odbc/odbc_impl/odbc_statement.cc | 6 +++--- .../sql/odbc/odbc_impl/spi/connection.h | 4 ++-- .../flight/sql/odbc/odbc_impl/spi/statement.h | 8 ++++---- .../arrow/flight/sql/odbc/odbc_impl/types.h | 6 +++--- .../arrow/flight/sql/odbc/odbc_impl/util.cc | 18 +++++++++--------- .../arrow/flight/sql/odbc/odbc_impl/util.h | 12 ++++++------ testing | 2 +- 16 files changed, 77 insertions(+), 56 deletions(-) diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/blocking_queue.h b/cpp/src/arrow/flight/sql/odbc/odbc_impl/blocking_queue.h index 5c9e6609d58..e52c305e461 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/blocking_queue.h +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/blocking_queue.h @@ -18,9 +18,9 @@ #pragma once #include -#include #include #include +#include #include #include @@ -43,7 +43,7 @@ class BlockingQueue { std::atomic closed_{false}; public: - typedef std::function(void)> Supplier; + typedef std::function(void)> Supplier; explicit BlockingQueue(size_t capacity) : capacity_(capacity), buffer_(capacity) {} @@ -58,7 +58,7 @@ class BlockingQueue { // Only one thread at a time be notified and call supplier auto item = supplier(); - if (!item) break; + if (!item.has_value()) break; Push(*item); } diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_auth_method.cc b/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_auth_method.cc index bdf7f71589c..6abede2abe2 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_auth_method.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_auth_method.cc @@ -24,6 +24,7 @@ #include "arrow/result.h" #include "arrow/status.h" +#include #include namespace arrow::flight::sql::odbc { @@ -63,7 +64,7 @@ class UserPasswordAuthMethod : public FlightSqlAuthMethod { void Authenticate(FlightSqlConnection& connection, FlightCallOptions& call_options) override { FlightCallOptions auth_call_options; - const boost::optional& login_timeout = + const std::optional& login_timeout = connection.GetAttribute(Connection::LOGIN_TIMEOUT); if (login_timeout && boost::get(*login_timeout) > 0) { // ODBC's LOGIN_TIMEOUT attribute and FlightCallOptions.timeout use 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 e18a58d069f..a53ecf89d1f 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 @@ -31,7 +31,6 @@ #include #include #include -#include #include "arrow/flight/sql/odbc/odbc_impl/exceptions.h" #include @@ -199,7 +198,7 @@ void FlightSqlConnection::PopulateMetadataSettings( metadata_settings_.chunk_buffer_capacity = GetChunkBufferCapacity(conn_property_map); } -boost::optional FlightSqlConnection::GetStringColumnLength( +std::optional FlightSqlConnection::GetStringColumnLength( const Connection::ConnPropertyMap& conn_property_map) { const int32_t min_string_column_length = 1; @@ -214,7 +213,7 @@ boost::optional FlightSqlConnection::GetStringColumnLength( "01000", ODBCErrorCodes_GENERAL_WARNING); } - return boost::none; + return std::nullopt; } bool FlightSqlConnection::GetUseWideChar(const ConnPropertyMap& conn_property_map) { @@ -250,7 +249,7 @@ const FlightCallOptions& FlightSqlConnection::PopulateCallOptions( const ConnPropertyMap& props) { // Set CONNECTION_TIMEOUT attribute or LOGIN_TIMEOUT depending on if this // is the first request. - const boost::optional& connection_timeout = + const std::optional& connection_timeout = closed_ ? GetAttribute(LOGIN_TIMEOUT) : GetAttribute(CONNECTION_TIMEOUT); if (connection_timeout && boost::get(*connection_timeout) > 0) { call_options_.timeout = @@ -388,17 +387,21 @@ bool FlightSqlConnection::SetAttribute(Connection::AttributeId attribute, } } -boost::optional FlightSqlConnection::GetAttribute( +std::optional FlightSqlConnection::GetAttribute( Connection::AttributeId attribute) { switch (attribute) { case ACCESS_MODE: // FlightSQL does not provide this metadata. - return boost::make_optional(Attribute(static_cast(SQL_MODE_READ_WRITE))); + return std::make_optional(Attribute(static_cast(SQL_MODE_READ_WRITE))); case PACKET_SIZE: - return boost::make_optional(Attribute(static_cast(0))); + return std::make_optional(Attribute(static_cast(0))); default: const auto& it = attribute_.find(attribute); - return boost::make_optional(it != attribute_.end(), it->second); + if (it != attribute_.end()) { + return std::make_optional(it->second); + } else { + return std::nullopt; + } } } diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_connection.h b/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_connection.h index 6219bb287e4..2561ea492f0 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_connection.h +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_connection.h @@ -19,6 +19,7 @@ #include "arrow/flight/sql/odbc/odbc_impl/spi/connection.h" +#include #include #include "arrow/flight/api.h" #include "arrow/flight/sql/api.h" @@ -84,7 +85,7 @@ class FlightSqlConnection : public Connection { bool SetAttribute(AttributeId attribute, const Attribute& value) override; - boost::optional GetAttribute( + std::optional GetAttribute( Connection::AttributeId attribute) override; Info GetInfo(uint16_t info_type) override; @@ -111,8 +112,7 @@ class FlightSqlConnection : public Connection { /// \note Visible for testing void SetClosed(bool is_closed); - boost::optional GetStringColumnLength( - const ConnPropertyMap& conn_property_map); + std::optional GetStringColumnLength(const ConnPropertyMap& conn_property_map); bool GetUseWideChar(const ConnPropertyMap& conn_property_map); diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_connection_test.cc b/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_connection_test.cc index 9c9b0f8f3c1..87ae526f158 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_connection_test.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_connection_test.cc @@ -21,6 +21,8 @@ #include "arrow/flight/types.h" #include "gtest/gtest.h" +#include + namespace arrow::flight::sql::odbc { TEST(AttributeTests, SetAndGetAttribute) { @@ -28,7 +30,7 @@ TEST(AttributeTests, SetAndGetAttribute) { connection.SetClosed(false); connection.SetAttribute(Connection::CONNECTION_TIMEOUT, static_cast(200)); - const boost::optional first_value = + const std::optional first_value = connection.GetAttribute(Connection::CONNECTION_TIMEOUT); EXPECT_TRUE(first_value); @@ -37,7 +39,7 @@ TEST(AttributeTests, SetAndGetAttribute) { connection.SetAttribute(Connection::CONNECTION_TIMEOUT, static_cast(300)); - const boost::optional change_value = + const std::optional change_value = connection.GetAttribute(Connection::CONNECTION_TIMEOUT); EXPECT_TRUE(change_value); @@ -49,7 +51,7 @@ TEST(AttributeTests, SetAndGetAttribute) { TEST(AttributeTests, GetAttributeWithoutSetting) { FlightSqlConnection connection(OdbcVersion::V_3); - const boost::optional optional = + const std::optional optional = connection.GetAttribute(Connection::CONNECTION_TIMEOUT); connection.SetClosed(false); @@ -72,7 +74,7 @@ TEST(MetadataSettingsTest, StringColumnLengthTest) { std::to_string(expected_string_column_length)}, }; - const boost::optional actual_string_column_length = + const std::optional actual_string_column_length = connection.GetStringColumnLength(properties); EXPECT_TRUE(actual_string_column_length); diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_statement.cc b/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_statement.cc index 30eb1fdf44a..a296e4cd489 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_statement.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_statement.cc @@ -29,7 +29,7 @@ #include "arrow/flight/sql/odbc/odbc_impl/util.h" #include "arrow/io/memory.h" -#include +#include #include #include "arrow/flight/sql/odbc/odbc_impl/exceptions.h" @@ -89,13 +89,17 @@ bool FlightSqlStatement::SetAttribute(StatementAttributeId attribute, } } -boost::optional FlightSqlStatement::GetAttribute( +std::optional FlightSqlStatement::GetAttribute( StatementAttributeId attribute) { const auto& it = attribute_.find(attribute); - return boost::make_optional(it != attribute_.end(), it->second); + if (it != attribute_.end()) { + return std::make_optional(it->second); + } else { + return std::nullopt; + } } -boost::optional> FlightSqlStatement::Prepare( +std::optional> FlightSqlStatement::Prepare( const std::string& query) { ClosePreparedStatementIfAny(prepared_statement_); @@ -107,7 +111,7 @@ boost::optional> FlightSqlStatement::Prepare( const auto& result_set_metadata = std::make_shared( prepared_statement_->dataset_schema(), metadata_settings_); - return boost::optional>(result_set_metadata); + return std::optional>(result_set_metadata); } bool FlightSqlStatement::ExecutePrepared() { diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_statement.h b/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_statement.h index 36dc245c1d7..211f4265da7 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_statement.h +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_statement.h @@ -26,6 +26,8 @@ #include "arrow/flight/sql/api.h" #include "arrow/flight/types.h" +#include + namespace arrow::flight::sql::odbc { class FlightSqlStatement : public Statement { @@ -51,9 +53,9 @@ class FlightSqlStatement : public Statement { bool SetAttribute(StatementAttributeId attribute, const Attribute& value) override; - boost::optional GetAttribute(StatementAttributeId attribute) override; + std::optional GetAttribute(StatementAttributeId attribute) override; - boost::optional> Prepare( + std::optional> Prepare( const std::string& query) override; bool ExecutePrepared() override; diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_stream_chunk_buffer.cc b/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_stream_chunk_buffer.cc index 25bf04ea507..70aedfdf03f 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_stream_chunk_buffer.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_stream_chunk_buffer.cc @@ -39,7 +39,15 @@ FlightStreamChunkBuffer::FlightStreamChunkBuffer( bool is_not_ok = !result.ok(); bool is_not_empty = result.ok() && (result.ValueOrDie().data != nullptr); - return boost::make_optional(is_not_ok || is_not_empty, std::move(result)); + // If result is valid, save the temp Flight SQL Client for future stream reader + // call. temp_flight_sql_client is intentionally null if the list of endpoint + // Locations is empty. + // After all data is fetched from reader, the temp client is closed. + if (is_not_ok || is_not_empty) { + return std::make_optional(std::move(result)); + } else { + return std::optional>{}; + } }; queue_.AddProducer(std::move(supplier)); } diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/odbc_connection.cc b/cpp/src/arrow/flight/sql/odbc/odbc_impl/odbc_connection.cc index ead2beada4b..a8f9417a085 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/odbc_connection.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/odbc_connection.cc @@ -36,6 +36,7 @@ #include #include #include +#include #include using ODBC::ODBCConnection; @@ -531,7 +532,7 @@ void ODBCConnection::SetConnectAttr(SQLINTEGER attribute, SQLPOINTER value, void ODBCConnection::GetConnectAttr(SQLINTEGER attribute, SQLPOINTER value, SQLINTEGER buffer_length, SQLINTEGER* output_length, bool is_unicode) { - boost::optional spi_attribute; + std::optional spi_attribute; switch (attribute) { // Internal connection attributes diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/odbc_statement.cc b/cpp/src/arrow/flight/sql/odbc/odbc_impl/odbc_statement.cc index d452e77db1d..ce1a99fa219 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/odbc_statement.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/odbc_statement.cc @@ -29,8 +29,8 @@ #include #include #include -#include #include +#include #include using ODBC::DescriptorRecord; @@ -273,7 +273,7 @@ void ODBCStatement::CopyAttributesFromConnection(ODBCConnection& connection) { bool ODBCStatement::IsPrepared() const { return is_prepared_; } void ODBCStatement::Prepare(const std::string& query) { - boost::optional > metadata = + std::optional > metadata = spi_statement_->Prepare(query); if (metadata) { @@ -352,7 +352,7 @@ bool ODBCStatement::Fetch(size_t rows) { void ODBCStatement::GetStmtAttr(SQLINTEGER statement_attribute, SQLPOINTER output, SQLINTEGER buffer_size, SQLINTEGER* str_len_ptr, bool is_unicode) { - boost::optional spi_attribute; + std::optional spi_attribute; switch (statement_attribute) { // Descriptor accessor attributes case SQL_ATTR_APP_PARAM_DESC: diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/spi/connection.h b/cpp/src/arrow/flight/sql/odbc/odbc_impl/spi/connection.h index 7a8243e7859..6e913cf2dba 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/spi/connection.h +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/spi/connection.h @@ -18,10 +18,10 @@ #pragma once #include -#include #include #include #include +#include #include #include @@ -88,7 +88,7 @@ class Connection { /// \brief Retrieve a connection attribute /// \param attribute [in] Attribute to be retrieved. - virtual boost::optional GetAttribute( + virtual std::optional GetAttribute( Connection::AttributeId attribute) = 0; /// \brief Retrieves info from the database (see ODBC's SQLGetInfo). diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/spi/statement.h b/cpp/src/arrow/flight/sql/odbc/odbc_impl/spi/statement.h index 970e447dfdc..d8b8daf1ec4 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/spi/statement.h +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/spi/statement.h @@ -17,14 +17,14 @@ #pragma once -#include #include #include +#include #include namespace arrow::flight::sql::odbc { -using boost::optional; +using std::optional; class ResultSet; @@ -74,9 +74,9 @@ class Statement { /// \brief Prepares the statement. /// Returns ResultSetMetadata if query returns a result set, - /// otherwise it returns `boost::none`. + /// otherwise it returns `std::nullopt`. /// \param query The SQL query to prepare. - virtual boost::optional> Prepare( + virtual std::optional> Prepare( const std::string& query) = 0; /// \brief Execute the prepared statement. diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/types.h b/cpp/src/arrow/flight/sql/odbc/odbc_impl/types.h index 7a91221cd44..e9817fa2387 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/types.h +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/types.h @@ -17,10 +17,10 @@ #pragma once +#include +#include #include "arrow/flight/sql/odbc/odbc_impl/platform.h" -#include - namespace arrow::flight::sql::odbc { /// \brief Supported ODBC versions. @@ -172,7 +172,7 @@ enum RowStatus : uint16_t { }; struct MetadataSettings { - boost::optional string_column_length{boost::none}; + std::optional string_column_length{std::nullopt}; size_t chunk_buffer_capacity; bool use_wide_char; }; diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/util.cc b/cpp/src/arrow/flight/sql/odbc/odbc_impl/util.cc index 59ee7dda565..d61a4638f36 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/util.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/util.cc @@ -1097,30 +1097,30 @@ int32_t GetDecimalTypePrecision(const std::shared_ptr& decimal_type) { return decimal128_type->precision(); } -boost::optional AsBool(const std::string& value) { +std::optional AsBool(const std::string& value) { if (boost::iequals(value, "true") || boost::iequals(value, "1")) { return true; } else if (boost::iequals(value, "false") || boost::iequals(value, "0")) { return false; } else { - return boost::none; + return std::nullopt; } } -boost::optional AsBool(const Connection::ConnPropertyMap& conn_property_map, - std::string_view property_name) { +std::optional AsBool(const Connection::ConnPropertyMap& conn_property_map, + std::string_view property_name) { auto extracted_property = conn_property_map.find(property_name); if (extracted_property != conn_property_map.end()) { return AsBool(extracted_property->second); } - return boost::none; + return std::nullopt; } -boost::optional AsInt32(int32_t min_value, - const Connection::ConnPropertyMap& conn_property_map, - std::string_view property_name) { +std::optional AsInt32(int32_t min_value, + const Connection::ConnPropertyMap& conn_property_map, + std::string_view property_name) { auto extracted_property = conn_property_map.find(property_name); if (extracted_property != conn_property_map.end()) { @@ -1130,7 +1130,7 @@ boost::optional AsInt32(int32_t min_value, return string_column_length; } } - return boost::none; + return std::nullopt; } } // namespace util diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/util.h b/cpp/src/arrow/flight/sql/odbc/odbc_impl/util.h index c17e77e7de8..21ee3f62fe5 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/util.h +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/util.h @@ -136,15 +136,15 @@ int32_t GetDecimalTypePrecision(const std::shared_ptr& decimal_type); /// Parse a string value to a boolean. /// \param value the value to be parsed. /// \return the parsed valued. -boost::optional AsBool(const std::string& value); +std::optional AsBool(const std::string& value); /// Looks up for a value inside the ConnPropertyMap and then try to parse it. /// In case it does not find or it cannot parse, the default value will be returned. /// \param conn_property_map the map with the connection properties. /// \param property_name the name of the property that will be looked up. /// \return the parsed valued. -boost::optional AsBool(const Connection::ConnPropertyMap& conn_property_map, - std::string_view property_name); +std::optional AsBool(const Connection::ConnPropertyMap& conn_property_map, + std::string_view property_name); /// Looks up for a value inside the ConnPropertyMap and then try to parse it. /// In case it does not find or it cannot parse, the default value will be returned. @@ -154,9 +154,9 @@ boost::optional AsBool(const Connection::ConnPropertyMap& conn_property_ma /// looked up. \return the parsed valued. \exception /// std::invalid_argument exception from std::stoi \exception /// std::out_of_range exception from std::stoi -boost::optional AsInt32(int32_t min_value, - const Connection::ConnPropertyMap& conn_property_map, - std::string_view property_name); +std::optional AsInt32(int32_t min_value, + const Connection::ConnPropertyMap& conn_property_map, + std::string_view property_name); } // namespace util } // namespace arrow::flight::sql::odbc diff --git a/testing b/testing index 9a56b4747da..8da05fdd62a 160000 --- a/testing +++ b/testing @@ -1 +1 @@ -Subproject commit 9a56b4747da5539e41b25a4d4827d0ed801ecd30 +Subproject commit 8da05fdd62a7243ef77aa9757acb62e0586a4d0c From d45d2f7745839a099e006cef4bfd461e64d6c478 Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Mon, 10 Nov 2025 09:45:24 -0800 Subject: [PATCH 19/35] Attempt to resolve conflict with sql/types.h Attempt to include Arrow headers before ODBC headers to avoid conflict with arrow/flight/sql/types.h --- cpp/src/arrow/flight/sql/odbc/CMakeLists.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cpp/src/arrow/flight/sql/odbc/CMakeLists.txt b/cpp/src/arrow/flight/sql/odbc/CMakeLists.txt index ac18a9bc7cd..053d7ce8058 100644 --- a/cpp/src/arrow/flight/sql/odbc/CMakeLists.txt +++ b/cpp/src/arrow/flight/sql/odbc/CMakeLists.txt @@ -38,7 +38,9 @@ add_subdirectory(tests) arrow_install_all_headers("arrow/flight/sql/odbc") -set(ARROW_FLIGHT_SQL_ODBC_SRCS entry_points.cc odbc_api.cc) +# Include Arrow headers before ODBC headers +# Compile entry_points.cc before odbc_api.cc due to conflict from sql.h and flight/sql/types.h +set(ARROW_FLIGHT_SQL_ODBC_SRCS odbc_api.cc entry_points.cc) if(WIN32) list(APPEND ARROW_FLIGHT_SQL_ODBC_SRCS odbc.def) From 73a92eefd0afc4bed13ee41100e61cc5f1b90bc7 Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Mon, 10 Nov 2025 13:08:15 -0800 Subject: [PATCH 20/35] Add `#pragma once` to odbc_test_suite.h To avoid redefinition error --- cpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.h b/cpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.h index e043a459f0a..823fc3d4614 100644 --- a/cpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.h +++ b/cpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.h @@ -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" From bf6b69ca774f97f6d69205810be07501d741cee1 Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Mon, 10 Nov 2025 13:20:14 -0800 Subject: [PATCH 21/35] Undefine duplicates in SQLGetInfo --- cpp/src/arrow/flight/sql/odbc/CMakeLists.txt | 4 +- .../sql/odbc/odbc_impl/get_info_cache.cc | 6 +- .../sql/odbc/odbc_impl/odbc_connection.cc | 1 + cpp/src/arrow/flight/sql/sql_info_undef.h | 174 ++++++++++++++++++ cpp/src/arrow/flight/sql/types.h | 1 + 5 files changed, 181 insertions(+), 5 deletions(-) create mode 100644 cpp/src/arrow/flight/sql/sql_info_undef.h diff --git a/cpp/src/arrow/flight/sql/odbc/CMakeLists.txt b/cpp/src/arrow/flight/sql/odbc/CMakeLists.txt index 053d7ce8058..ac18a9bc7cd 100644 --- a/cpp/src/arrow/flight/sql/odbc/CMakeLists.txt +++ b/cpp/src/arrow/flight/sql/odbc/CMakeLists.txt @@ -38,9 +38,7 @@ add_subdirectory(tests) arrow_install_all_headers("arrow/flight/sql/odbc") -# Include Arrow headers before ODBC headers -# Compile entry_points.cc before odbc_api.cc due to conflict from sql.h and flight/sql/types.h -set(ARROW_FLIGHT_SQL_ODBC_SRCS odbc_api.cc entry_points.cc) +set(ARROW_FLIGHT_SQL_ODBC_SRCS entry_points.cc odbc_api.cc) if(WIN32) list(APPEND ARROW_FLIGHT_SQL_ODBC_SRCS odbc.def) diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/get_info_cache.cc b/cpp/src/arrow/flight/sql/odbc/odbc_impl/get_info_cache.cc index bf2f6b6eca2..30254c8f53b 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/get_info_cache.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/get_info_cache.cc @@ -19,8 +19,6 @@ #include "arrow/flight/sql/odbc/odbc_impl/platform.h" -#include -#include #include "arrow/array.h" #include "arrow/array/array_nested.h" #include "arrow/flight/sql/api.h" @@ -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 +#include + // 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 diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/odbc_connection.cc b/cpp/src/arrow/flight/sql/odbc/odbc_impl/odbc_connection.cc index a8f9417a085..1d4617ca1bc 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/odbc_connection.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/odbc_connection.cc @@ -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 #include #include diff --git a/cpp/src/arrow/flight/sql/sql_info_undef.h b/cpp/src/arrow/flight/sql/sql_info_undef.h new file mode 100644 index 00000000000..c3903e418fe --- /dev/null +++ b/cpp/src/arrow/flight/sql/sql_info_undef.h @@ -0,0 +1,174 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#pragma once + +/// \brief Undefine the ODBC macros in sql.h and sqlext.h to avoid conflicts with ODBC +/// library +/// +/// This file is to resolve build issues from linking. Should not be used with sql.h or +/// sql_ext.h + +#ifdef SQL_IDENTIFIER_CASE +# undef SQL_IDENTIFIER_CASE +#endif // SQL_IDENTIFIER_CASE + +#ifdef SQL_IDENTIFIER_QUOTE_CHAR +# undef SQL_IDENTIFIER_QUOTE_CHAR +#endif // SQL_IDENTIFIER_QUOTE_CHAR + +#ifdef SQL_QUOTED_IDENTIFIER_CASE +# undef SQL_QUOTED_IDENTIFIER_CASE +#endif // SQL_QUOTED_IDENTIFIER_CASE + +#ifdef SQL_KEYWORDS +# undef SQL_KEYWORDS +#endif // SQL_KEYWORDS + +#ifdef SQL_NUMERIC_FUNCTIONS +# undef SQL_NUMERIC_FUNCTIONS +#endif // SQL_NUMERIC_FUNCTIONS + +#ifdef SQL_STRING_FUNCTIONS +# undef SQL_STRING_FUNCTIONS +#endif // SQL_STRING_FUNCTIONS + +#ifdef SQL_SYSTEM_FUNCTIONS +# undef SQL_SYSTEM_FUNCTIONS +#endif // SQL_SYSTEM_FUNCTIONS + +#ifdef SQL_SCHEMA_TERM +# undef SQL_SCHEMA_TERM +#endif // SQL_SCHEMA_TERM + +#ifdef SQL_PROCEDURE_TERM +# undef SQL_PROCEDURE_TERM +#endif // SQL_PROCEDURE_TERM + +#ifdef SQL_CATALOG_TERM +# undef SQL_CATALOG_TERM +#endif // SQL_CATALOG_TERM + +#ifdef SQL_MAX_COLUMNS_IN_GROUP_BY +# undef SQL_MAX_COLUMNS_IN_GROUP_BY +#endif // SQL_MAX_COLUMNS_IN_GROUP_BY + +#ifdef SQL_MAX_COLUMNS_IN_INDEX +# undef SQL_MAX_COLUMNS_IN_INDEX +#endif // SQL_MAX_COLUMNS_IN_INDEX + +#ifdef SQL_MAX_COLUMNS_IN_ORDER_BY +# undef SQL_MAX_COLUMNS_IN_ORDER_BY +#endif // SQL_MAX_COLUMNS_IN_ORDER_BY + +#ifdef SQL_MAX_COLUMNS_IN_SELECT +# undef SQL_MAX_COLUMNS_IN_SELECT +#endif // SQL_MAX_COLUMNS_IN_SELECT + +#ifdef SQL_MAX_COLUMNS_IN_TABLE +# undef SQL_MAX_COLUMNS_IN_TABLE +#endif // SQL_MAX_COLUMNS_IN_TABLE + +#ifdef SQL_MAX_ROW_SIZE +# undef SQL_MAX_ROW_SIZE +#endif // SQL_MAX_ROW_SIZE + +#ifdef SQL_MAX_TABLES_IN_SELECT +# undef SQL_MAX_TABLES_IN_SELECT +#endif // SQL_MAX_TABLES_IN_SELECT + +// ---- Conversion Info ---- + +#ifdef SQL_CONVERT_BIGINT +# undef SQL_CONVERT_BIGINT +#endif // SQL_CONVERT_BIGINT + +#ifdef SQL_CONVERT_BINARY +# undef SQL_CONVERT_BINARY +#endif // SQL_CONVERT_BINARY + +#ifdef SQL_CONVERT_BIT +# undef SQL_CONVERT_BIT +#endif // SQL_CONVERT_BIT + +#ifdef SQL_CONVERT_CHAR +# undef SQL_CONVERT_CHAR +#endif // SQL_CONVERT_CHAR + +#ifdef SQL_CONVERT_DATE +# undef SQL_CONVERT_DATE +#endif // SQL_CONVERT_DATE + +#ifdef SQL_CONVERT_DECIMAL +# undef SQL_CONVERT_DECIMAL +#endif // SQL_CONVERT_DECIMAL + +#ifdef SQL_CONVERT_FLOAT +# undef SQL_CONVERT_FLOAT +#endif // SQL_CONVERT_FLOAT + +#ifdef SQL_CONVERT_INTEGER +# undef SQL_CONVERT_INTEGER +#endif // SQL_CONVERT_INTEGER + +#ifdef SQL_CONVERT_INTERVAL_DAY_TIME +# undef SQL_CONVERT_INTERVAL_DAY_TIME +#endif // SQL_CONVERT_INTERVAL_DAY_TIME + +#ifdef SQL_CONVERT_INTERVAL_YEAR_MONTH +# undef SQL_CONVERT_INTERVAL_YEAR_MONTH +#endif // SQL_CONVERT_INTERVAL_YEAR_MONTH + +#ifdef SQL_CONVERT_LONGVARBINARY +# undef SQL_CONVERT_LONGVARBINARY +#endif // SQL_CONVERT_LONGVARBINARY + +#ifdef SQL_CONVERT_LONGVARCHAR +# undef SQL_CONVERT_LONGVARCHAR +#endif // SQL_CONVERT_LONGVARCHAR + +#ifdef SQL_CONVERT_NUMERIC +# undef SQL_CONVERT_NUMERIC +#endif // SQL_CONVERT_NUMERIC + +#ifdef SQL_CONVERT_REAL +# undef SQL_CONVERT_REAL +#endif // SQL_CONVERT_REAL + +#ifdef SQL_CONVERT_SMALLINT +# undef SQL_CONVERT_SMALLINT +#endif // SQL_CONVERT_SMALLINT + +#ifdef SQL_CONVERT_TIME +# undef SQL_CONVERT_TIME +#endif // SQL_CONVERT_TIME + +#ifdef SQL_CONVERT_TIMESTAMP +# undef SQL_CONVERT_TIMESTAMP +#endif // SQL_CONVERT_TIMESTAMP + +#ifdef SQL_CONVERT_TINYINT +# undef SQL_CONVERT_TINYINT +#endif // SQL_CONVERT_TINYINT + +#ifdef SQL_CONVERT_VARBINARY +# undef SQL_CONVERT_VARBINARY +#endif // SQL_CONVERT_VARBINARY + +#ifdef SQL_CONVERT_VARCHAR +# undef SQL_CONVERT_VARCHAR +#endif // SQL_CONVERT_VARCHAR diff --git a/cpp/src/arrow/flight/sql/types.h b/cpp/src/arrow/flight/sql/types.h index fe90c08ed20..e79e4d66e60 100644 --- a/cpp/src/arrow/flight/sql/types.h +++ b/cpp/src/arrow/flight/sql/types.h @@ -26,6 +26,7 @@ #include #include "arrow/flight/sql/visibility.h" +#include "arrow/flight/sql/sql_info_undef.h" #include "arrow/type_fwd.h" namespace arrow { From 63cd922cba5807d4c8031d3a3910a4b6b81937a7 Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Mon, 10 Nov 2025 16:11:52 -0800 Subject: [PATCH 22/35] Add `sql_info_undef.h` to sqlite_sql_info.cc --- cpp/src/arrow/flight/sql/example/sqlite_sql_info.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cpp/src/arrow/flight/sql/example/sqlite_sql_info.cc b/cpp/src/arrow/flight/sql/example/sqlite_sql_info.cc index 9737b5a3090..946069070f6 100644 --- a/cpp/src/arrow/flight/sql/example/sqlite_sql_info.cc +++ b/cpp/src/arrow/flight/sql/example/sqlite_sql_info.cc @@ -20,6 +20,8 @@ #include "arrow/flight/sql/types.h" #include "arrow/util/config.h" +#include "arrow/flight/sql/sql_info_undef.h" + namespace arrow { namespace flight { namespace sql { From 8d1d56108bb44f2cd37258a3bbc9af649ab3aa0a Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Mon, 10 Nov 2025 16:37:31 -0800 Subject: [PATCH 23/35] Fix lint --- cpp/src/arrow/flight/sql/types.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/arrow/flight/sql/types.h b/cpp/src/arrow/flight/sql/types.h index e79e4d66e60..14a323828b5 100644 --- a/cpp/src/arrow/flight/sql/types.h +++ b/cpp/src/arrow/flight/sql/types.h @@ -25,8 +25,8 @@ #include #include -#include "arrow/flight/sql/visibility.h" #include "arrow/flight/sql/sql_info_undef.h" +#include "arrow/flight/sql/visibility.h" #include "arrow/type_fwd.h" namespace arrow { From 53a04f3d7f449710bedca7912a4ad439c3490a0f Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Mon, 10 Nov 2025 16:39:02 -0800 Subject: [PATCH 24/35] Remove debug messages --- cpp/cmake_modules/ThirdpartyToolchain.cmake | 5 ----- 1 file changed, 5 deletions(-) diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index 7d87b639b98..1724c0d3a3d 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -1269,19 +1269,14 @@ else() endif() if(ARROW_USE_BOOST) - message(status "-AL- ARROW_BOOST_USE_SHARED: ${ARROW_BOOST_USE_SHARED}") if(ARROW_BOOST_USE_SHARED) # Find shared Boost libraries. set(Boost_USE_STATIC_LIBS OFF) set(BUILD_SHARED_LIBS_KEEP ${BUILD_SHARED_LIBS}) set(BUILD_SHARED_LIBS ON) - message(status "-AL- ARROW_BOOST_USE_SHARED TEMP - Boost_USE_STATIC_LIBS: ${Boost_USE_STATIC_LIBS}" - ) else() # Find static boost headers and libs set(Boost_USE_STATIC_LIBS ON) - message(status "-AL- NOT ARROW_BOOST_USE_SHARED TEMP - Boost_USE_STATIC_LIBS: ${Boost_USE_STATIC_LIBS}" - ) endif() if(ARROW_BOOST_REQUIRE_LIBRARY) set(ARROW_BOOST_COMPONENTS filesystem) From 8af22dcab973c13130946b935b87093e0765a39c Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Wed, 12 Nov 2025 10:37:38 -0800 Subject: [PATCH 25/35] Disable `UNITY_BUILD` for ODBC test due to conflict on `sqlite_sql_info.cc` On some workflows, unity build is set to ON to make the build faster. This is an attempt to resolve the build issue. --- cpp/src/arrow/flight/sql/odbc/tests/CMakeLists.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cpp/src/arrow/flight/sql/odbc/tests/CMakeLists.txt b/cpp/src/arrow/flight/sql/odbc/tests/CMakeLists.txt index 4bc240637e7..fc192ad12db 100644 --- a/cpp/src/arrow/flight/sql/odbc/tests/CMakeLists.txt +++ b/cpp/src/arrow/flight/sql/odbc/tests/CMakeLists.txt @@ -44,3 +44,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) From 588fc9db6ebb9978b4523b5bdcf78ac2f1896f16 Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Wed, 12 Nov 2025 13:39:39 -0800 Subject: [PATCH 26/35] Attempt to resolve `arrow-compute-grouper-benchmark` build issue I think `if(ARROW_FLIGHT_TEST_LINKAGE STREQUAL "static")` might be more appropriate here, as I am seeing build issues due to both dynamic and static linking occurring. I see in https://github.com/apache/arrow/commit/59903d089ec5b1766e1622d94c1f618b539b205d, this check is used for `arrow-filesystem-s3fs-benchmark` --- cpp/src/arrow/compute/row/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/arrow/compute/row/CMakeLists.txt b/cpp/src/arrow/compute/row/CMakeLists.txt index 542dc314806..6ad74cbe9fd 100644 --- a/cpp/src/arrow/compute/row/CMakeLists.txt +++ b/cpp/src/arrow/compute/row/CMakeLists.txt @@ -22,7 +22,7 @@ arrow_install_all_headers("arrow/compute/row") if(ARROW_BUILD_BENCHMARKS AND ARROW_COMPUTE) add_arrow_benchmark(grouper_benchmark PREFIX "arrow-compute") - if(ARROW_BUILD_STATIC) + if(ARROW_TEST_LINKAGE STREQUAL "static") target_link_libraries(arrow-compute-grouper-benchmark PUBLIC arrow_compute_static) else() target_link_libraries(arrow-compute-grouper-benchmark PUBLIC arrow_compute_shared) From 3030aa0034b47c42c563ca46c99cf08e44ce0a7f Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Wed, 12 Nov 2025 16:26:45 -0800 Subject: [PATCH 27/35] Link ODBC library on all platforms On my local MSVC Windows machine, Visual Studio is used to build without needing to link ` ${ODBCINST}` explicitly, its behavior might be different from Ninja which is what CI uses. `${ODBCINST}` is likely needed by Linux as well, so adding it for all platforms. --- cpp/src/arrow/flight/sql/odbc/odbc_impl/CMakeLists.txt | 10 +++------- cpp/src/arrow/flight/sql/sql_info_undef.h | 2 -- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/CMakeLists.txt b/cpp/src/arrow/flight/sql/odbc/odbc_impl/CMakeLists.txt index 8f09fccd71d..a820a3c468a 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/CMakeLists.txt +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/CMakeLists.txt @@ -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 diff --git a/cpp/src/arrow/flight/sql/sql_info_undef.h b/cpp/src/arrow/flight/sql/sql_info_undef.h index c3903e418fe..244ce5f080a 100644 --- a/cpp/src/arrow/flight/sql/sql_info_undef.h +++ b/cpp/src/arrow/flight/sql/sql_info_undef.h @@ -91,8 +91,6 @@ # undef SQL_MAX_TABLES_IN_SELECT #endif // SQL_MAX_TABLES_IN_SELECT -// ---- Conversion Info ---- - #ifdef SQL_CONVERT_BIGINT # undef SQL_CONVERT_BIGINT #endif // SQL_CONVERT_BIGINT From 4e9461df1ed42266988a32051a008ed628bb5fae Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Thu, 13 Nov 2025 09:53:27 -0800 Subject: [PATCH 28/35] Fix ODBC dll name Use `arrow_flight_sql_odbc.dll` instead of `libarrow_flight_sql_odbc.dll` which is the naming convention used on MinGW Windows --- .github/workflows/cpp_windows.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/cpp_windows.yml b/.github/workflows/cpp_windows.yml index cd8ad6af271..d98a79c607d 100644 --- a/.github/workflows/cpp_windows.yml +++ b/.github/workflows/cpp_windows.yml @@ -151,7 +151,7 @@ jobs: - 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 + call "cpp\src\arrow\flight\sql\odbc\tests\install_odbc.cmd" ${{ github.workspace }}\build\cpp\%ARROW_BUILD_TYPE%\arrow_flight_sql_odbc.dll - name: Test shell: cmd run: | From 69475da5e1e7349f7ef3c55c9d8e861d5911b0b7 Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Thu, 13 Nov 2025 13:59:40 -0800 Subject: [PATCH 29/35] Set `VCPKG_ROOT` in test phase --- .github/workflows/cpp_windows.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/cpp_windows.yml b/.github/workflows/cpp_windows.yml index d98a79c607d..1b4c82a31b2 100644 --- a/.github/workflows/cpp_windows.yml +++ b/.github/workflows/cpp_windows.yml @@ -155,7 +155,9 @@ jobs: - name: Test shell: cmd run: | + set VCPKG_ROOT_KEEP=%VCPKG_ROOT% call "C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Auxiliary\Build\vcvarsall.bat" ${{ inputs.arch }} # For ORC set TZDIR=${{ steps.setup-msys2.outputs.msys2-location }}\usr\share\zoneinfo + set VCPKG_ROOT=%VCPKG_ROOT_KEEP% bash -c "ci/scripts/cpp_test.sh $(pwd) $(pwd)/build" From 646f347ddab5767ad78078a0d1b670ac5453ca59 Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Thu, 13 Nov 2025 17:17:29 -0800 Subject: [PATCH 30/35] Convert VCPKG Windows path to MSYS path --- .github/workflows/cpp_windows.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/cpp_windows.yml b/.github/workflows/cpp_windows.yml index 1b4c82a31b2..2e92a319a33 100644 --- a/.github/workflows/cpp_windows.yml +++ b/.github/workflows/cpp_windows.yml @@ -159,5 +159,7 @@ jobs: call "C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Auxiliary\Build\vcvarsall.bat" ${{ inputs.arch }} # For ORC set TZDIR=${{ steps.setup-msys2.outputs.msys2-location }}\usr\share\zoneinfo - set VCPKG_ROOT=%VCPKG_ROOT_KEEP% + + # Convert VCPKG Windows path to MSYS path + for /f "usebackq delims=" %%I in (`bash -c "cygpath -u \"$VCPKG_ROOT_KEEP\""` ) do set VCPKG_ROOT=%%I bash -c "ci/scripts/cpp_test.sh $(pwd) $(pwd)/build" From 97b30d2910fa3e099397254afe077c68ff1793da Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Fri, 14 Nov 2025 12:12:15 -0800 Subject: [PATCH 31/35] Prepend vcpkg to search vcpkg before `/lib/cmake` Since we are installing dependencies on vcpkg, if `lib/cmake` is searched first, then cmake will look into that directory and use the wrong paths. --- ci/scripts/cpp_test.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ci/scripts/cpp_test.sh b/ci/scripts/cpp_test.sh index a0b77b11be2..729d2cea787 100755 --- a/ci/scripts/cpp_test.sh +++ b/ci/scripts/cpp_test.sh @@ -144,8 +144,9 @@ if [ "${ARROW_USE_MESON:-OFF}" = "OFF" ] && \ CMAKE_PREFIX_PATH+="/lib/cmake/" ;; esac + # Search vcpkg before /lib/cmake. if [ -n "${VCPKG_ROOT}" ] && [ -n "${VCPKG_DEFAULT_TRIPLET}" ]; then - CMAKE_PREFIX_PATH+=";${VCPKG_ROOT}/installed/${VCPKG_DEFAULT_TRIPLET}" + CMAKE_PREFIX_PATH="${VCPKG_ROOT}/installed/${VCPKG_DEFAULT_TRIPLET};${CMAKE_PREFIX_PATH}" fi cmake \ -S "${source_dir}/examples/minimal_build" \ From 763bbe529c81f47f4854dcdcab62c168a214dd95 Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Fri, 14 Nov 2025 16:22:15 -0800 Subject: [PATCH 32/35] Run ODBC test once outside of test script If ODBC test is run using MinGW Shell, segfault occurs. I am not getting any seg fault on my local MSVC Windows when I run the tests without the bash script. But if Windows CI breaks from running the standalone exe then I will look into this --- .github/workflows/cpp_windows.yml | 5 ++++- ci/scripts/cpp_test.sh | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/cpp_windows.yml b/.github/workflows/cpp_windows.yml index 2e92a319a33..959feaa26ef 100644 --- a/.github/workflows/cpp_windows.yml +++ b/.github/workflows/cpp_windows.yml @@ -162,4 +162,7 @@ jobs: # Convert VCPKG Windows path to MSYS path for /f "usebackq delims=" %%I in (`bash -c "cygpath -u \"$VCPKG_ROOT_KEEP\""` ) do set VCPKG_ROOT=%%I - bash -c "ci/scripts/cpp_test.sh $(pwd) $(pwd)/build" + + # Run ODBC test + ${{ github.workspace }}\build\cpp\%ARROW_BUILD_TYPE%\arrow-flight-sql-odbc-test.exe + diff --git a/ci/scripts/cpp_test.sh b/ci/scripts/cpp_test.sh index 729d2cea787..1e261e16e15 100755 --- a/ci/scripts/cpp_test.sh +++ b/ci/scripts/cpp_test.sh @@ -82,6 +82,7 @@ case "$(uname)" in exclude_tests+=("gandiva-precompiled-test") exclude_tests+=("gandiva-projector-test") exclude_tests+=("gandiva-utf8-test") + exclude_tests+=("arrow-flight-sql-odbc-test") ;; *) n_jobs=${NPROC:-1} From 6145edab06106727cc8eb68d7df3666d08942bbe Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Tue, 18 Nov 2025 11:11:05 -0800 Subject: [PATCH 33/35] Enable regular ctest tests --- .github/workflows/cpp_windows.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/cpp_windows.yml b/.github/workflows/cpp_windows.yml index 959feaa26ef..b4000adb909 100644 --- a/.github/workflows/cpp_windows.yml +++ b/.github/workflows/cpp_windows.yml @@ -163,6 +163,8 @@ jobs: # Convert VCPKG Windows path to MSYS path for /f "usebackq delims=" %%I in (`bash -c "cygpath -u \"$VCPKG_ROOT_KEEP\""` ) do set VCPKG_ROOT=%%I + bash -c "ci/scripts/cpp_test.sh $(pwd) $(pwd)/build" + # Run ODBC test ${{ github.workspace }}\build\cpp\%ARROW_BUILD_TYPE%\arrow-flight-sql-odbc-test.exe From dcfa2180ea1634a8d67dfd2ef7da96801986affc Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Tue, 18 Nov 2025 11:37:53 -0800 Subject: [PATCH 34/35] Draft enable ODBC global setup/teardown --- cpp/src/arrow/compute/registry.cc | 30 ++++++- cpp/src/arrow/compute/registry.h | 3 + .../flight/sql/odbc/tests/CMakeLists.txt | 1 + .../flight/sql/odbc/tests/odbc_test_suite.cc | 23 ++++- .../flight/sql/odbc/tests/odbc_test_util.h | 83 +++++++++++++++++++ 5 files changed, 134 insertions(+), 6 deletions(-) create mode 100644 cpp/src/arrow/flight/sql/odbc/tests/odbc_test_util.h diff --git a/cpp/src/arrow/compute/registry.cc b/cpp/src/arrow/compute/registry.cc index be0ebd32016..b73a79ff066 100644 --- a/cpp/src/arrow/compute/registry.cc +++ b/cpp/src/arrow/compute/registry.cc @@ -30,14 +30,31 @@ #include "arrow/util/config.h" // For ARROW_COMPUTE #include "arrow/util/logging.h" +#include // -AL- TEMP remove later + namespace arrow { namespace compute { class FunctionRegistry::FunctionRegistryImpl { public: explicit FunctionRegistryImpl(FunctionRegistryImpl* parent = NULLPTR) - : parent_(parent) {} - ~FunctionRegistryImpl() {} + : parent_(parent) {// -AL- TEMP remove later + std::cout << "-AL- FunctionRegistryImpl constructor called\n"; + } + ~FunctionRegistryImpl() { // -AL- TEMP remove later + size_t name_size = name_to_function_.size(); + std::cout << "-AL- ~FunctionRegistryImpl name_to_function_ size:" << name_size << + "\n"; + + //auto it = name_to_function_.find(std::string("split_pattern")); + //auto func = it->second; + //for (auto& kv : name_to_function_) { + // kv.second.reset(); + //} + //name_to_function_.clear(); + + std::cout << "-AL- ~FunctionRegistryImpl destructor called\n"; + } Status CanAddFunction(std::shared_ptr function, bool allow_overwrite) { if (parent_ != NULLPTR) { @@ -127,6 +144,9 @@ class FunctionRegistry::FunctionRegistryImpl { const Function* cast_function() { return cast_function_; } + void ClearFunctioRegistry() { name_to_function_.clear(); + } + private: // must not acquire mutex Status CanAddFunctionName(const std::string& name, bool allow_overwrite) { @@ -215,6 +235,7 @@ class FunctionRegistry::FunctionRegistryImpl { }; std::unique_ptr FunctionRegistry::Make() { + std::cout << "-AL- Make is called once TEMP \n"; return std::unique_ptr(new FunctionRegistry()); } @@ -277,9 +298,14 @@ int FunctionRegistry::num_functions() const { return impl_->num_functions(); } const Function* FunctionRegistry::cast_function() const { return impl_->cast_function(); } +void FunctionRegistry::ClearFunctioRegistry() { + impl_->ClearFunctioRegistry(); +} + namespace internal { static std::unique_ptr CreateBuiltInRegistry() { + std::cout << "-AL- CreateBuiltInRegistry is called once TEMP \n"; auto registry = FunctionRegistry::Make(); // Register core kernels diff --git a/cpp/src/arrow/compute/registry.h b/cpp/src/arrow/compute/registry.h index f31c4c1ba59..6ea8bb94f80 100644 --- a/cpp/src/arrow/compute/registry.h +++ b/cpp/src/arrow/compute/registry.h @@ -111,6 +111,9 @@ class ARROW_EXPORT FunctionRegistry { /// /// Helpful for get cast function as needed. const Function* cast_function() const; + + // -AL- todo docs? + void ClearFunctioRegistry(); private: FunctionRegistry(); diff --git a/cpp/src/arrow/flight/sql/odbc/tests/CMakeLists.txt b/cpp/src/arrow/flight/sql/odbc/tests/CMakeLists.txt index fc192ad12db..67c3f3d8dd1 100644 --- a/cpp/src/arrow/flight/sql/odbc/tests/CMakeLists.txt +++ b/cpp/src/arrow/flight/sql/odbc/tests/CMakeLists.txt @@ -34,6 +34,7 @@ add_arrow_test(flight_sql_odbc_test SOURCES odbc_test_suite.cc odbc_test_suite.h + odbc_test_util.h connection_test.cc # Enable Protobuf cleanup after test execution # GH-46889: move protobuf_test_util to a more common location diff --git a/cpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.cc b/cpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.cc index eb6c60b9762..17fcea154a1 100644 --- a/cpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.cc +++ b/cpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.cc @@ -20,6 +20,7 @@ #include "arrow/flight/sql/odbc/odbc_impl/flight_sql_connection.h" #include "arrow/flight/sql/odbc/tests/odbc_test_suite.h" +#include "arrow/flight/sql/odbc/tests/odbc_test_util.h" // For DSN registration #include "arrow/flight/sql/odbc/odbc_impl/config/configuration.h" @@ -28,6 +29,13 @@ namespace arrow::flight::sql::odbc { +// -AL- todo rename later +auto alinatest_env = ::testing::AddGlobalTestEnvironment(new AlinaTestEnvironment); + +AlinaTestEnvironment* GetAlinaTestEnv() { + return ::arrow::internal::checked_cast(alinatest_env); +} + void ODBCRemoteTestBase::AllocEnvConnHandles(SQLINTEGER odbc_ver) { // Allocate an environment handle ASSERT_EQ(SQL_SUCCESS, SQLAllocEnv(&env)); @@ -54,12 +62,16 @@ void ODBCRemoteTestBase::ConnectWithString(std::string connect_str) { SQLWCHAR out_str[kOdbcBufferSize]; SQLSMALLINT out_str_len; + // -AL- once local conn handles are removed,can rename back to conn + SQLHDBC global_conn = GetAlinaTestEnv()->getConnHandle(); + //SQLHDBC global_conn = conn; //-AL- TEMP + // Connecting to ODBC server. ASSERT_EQ(SQL_SUCCESS, - SQLDriverConnect(conn, NULL, &connect_str0[0], + SQLDriverConnect(global_conn, NULL, &connect_str0[0], static_cast(connect_str0.size()), out_str, kOdbcBufferSize, &out_str_len, SQL_DRIVER_NOPROMPT)) - << GetOdbcErrorMessage(SQL_HANDLE_DBC, conn); + << GetOdbcErrorMessage(SQL_HANDLE_DBC, global_conn); // GH-47710: TODO Allocate a statement using alloc handle // ASSERT_EQ(SQL_SUCCESS, SQLAllocHandle(SQL_HANDLE_STMT, conn, &stmt)); @@ -69,9 +81,12 @@ void ODBCRemoteTestBase::Disconnect() { // GH-47710: TODO Close statement // EXPECT_EQ(SQL_SUCCESS, SQLFreeHandle(SQL_HANDLE_STMT, stmt)); + // -AL- once local conn handles are removed,can rename back to conn + SQLHDBC global_conn = GetAlinaTestEnv()->getConnHandle(); + // Disconnect from ODBC - EXPECT_EQ(SQL_SUCCESS, SQLDisconnect(conn)) - << GetOdbcErrorMessage(SQL_HANDLE_DBC, conn); + EXPECT_EQ(SQL_SUCCESS, SQLDisconnect(global_conn)) + << GetOdbcErrorMessage(SQL_HANDLE_DBC, global_conn); FreeEnvConnHandles(); } diff --git a/cpp/src/arrow/flight/sql/odbc/tests/odbc_test_util.h b/cpp/src/arrow/flight/sql/odbc/tests/odbc_test_util.h new file mode 100644 index 00000000000..4ffbdb7159d --- /dev/null +++ b/cpp/src/arrow/flight/sql/odbc/tests/odbc_test_util.h @@ -0,0 +1,83 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#include + +#include "arrow/flight/sql/odbc/odbc_impl/platform.h" + +#include "arrow/compute/api.h" // -AL- attempt to keep registry alive + +#include +#include + +#include + +namespace arrow::flight::sql::odbc { + +// -AL- todo update to actual ODBC allocation +class AlinaTestEnvironment : public ::testing::Environment { + public: + void SetUp() override { + int x = 1; + // -AL- todo add env_v2 for v2 after global setup/teardown works. + + // Allocate an environment handle + ASSERT_EQ(SQL_SUCCESS, SQLAllocEnv(&env)); + + ASSERT_EQ(SQL_SUCCESS, + SQLSetEnvAttr( + env, SQL_ATTR_ODBC_VERSION, + reinterpret_cast(static_cast(SQL_OV_ODBC3)), 0)); + + // Allocate a connection using alloc handle + ASSERT_EQ(SQL_SUCCESS, SQLAllocHandle(SQL_HANDLE_DBC, env, &conn)); + std::cout << "-AL- AlinaTestEnvironment::SetUp\n"; + } + + void TearDown() override { + + + // -AL- this doesn't work + //// Remove function registry before test exits + auto reg = arrow::compute::GetFunctionRegistry(); + reg->ClearFunctioRegistry(); + // delete reg; + + + int y = 1; + // Free connection handle + EXPECT_EQ(SQL_SUCCESS, SQLFreeHandle(SQL_HANDLE_DBC, conn)); + + // Free environment handle + EXPECT_EQ(SQL_SUCCESS, SQLFreeHandle(SQL_HANDLE_ENV, env)); + std::cout << "-AL- AlinaTestEnvironment::TearDown\n"; + + } + + SQLHENV getEnvHandle() { return env; } + + SQLHDBC getConnHandle() { return conn; } + + private: + /** ODBC Environment. */ + SQLHENV env = 0; + + /** ODBC Connect. */ + SQLHDBC conn = 0; +}; + +} // namespace arrow::flight::sql::odbc From 92f434f4761e2ec4e5466ba3ff25e5012f0ac38e Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Tue, 18 Nov 2025 16:08:33 -0800 Subject: [PATCH 35/35] Code Clean up and enable ODBC tests in CI Still need to modify ODBCUtilEnvironment if we decide to use it. Still need to add ODBC V2 support so a different env and conn is used for ODBC 2 tests. --- .github/workflows/cpp_windows.yml | 4 --- ci/scripts/cpp_test.sh | 1 - cpp/src/arrow/compute/registry.cc | 30 +++---------------- cpp/src/arrow/compute/registry.h | 6 ++-- .../flight/sql/odbc/tests/odbc_test_suite.cc | 17 +++++------ .../flight/sql/odbc/tests/odbc_test_util.h | 18 ++++------- 6 files changed, 21 insertions(+), 55 deletions(-) diff --git a/.github/workflows/cpp_windows.yml b/.github/workflows/cpp_windows.yml index b4000adb909..5215c97f9ca 100644 --- a/.github/workflows/cpp_windows.yml +++ b/.github/workflows/cpp_windows.yml @@ -164,7 +164,3 @@ jobs: for /f "usebackq delims=" %%I in (`bash -c "cygpath -u \"$VCPKG_ROOT_KEEP\""` ) do set VCPKG_ROOT=%%I bash -c "ci/scripts/cpp_test.sh $(pwd) $(pwd)/build" - - # Run ODBC test - ${{ github.workspace }}\build\cpp\%ARROW_BUILD_TYPE%\arrow-flight-sql-odbc-test.exe - diff --git a/ci/scripts/cpp_test.sh b/ci/scripts/cpp_test.sh index 1e261e16e15..729d2cea787 100755 --- a/ci/scripts/cpp_test.sh +++ b/ci/scripts/cpp_test.sh @@ -82,7 +82,6 @@ case "$(uname)" in exclude_tests+=("gandiva-precompiled-test") exclude_tests+=("gandiva-projector-test") exclude_tests+=("gandiva-utf8-test") - exclude_tests+=("arrow-flight-sql-odbc-test") ;; *) n_jobs=${NPROC:-1} diff --git a/cpp/src/arrow/compute/registry.cc b/cpp/src/arrow/compute/registry.cc index b73a79ff066..9ada51611f4 100644 --- a/cpp/src/arrow/compute/registry.cc +++ b/cpp/src/arrow/compute/registry.cc @@ -30,31 +30,14 @@ #include "arrow/util/config.h" // For ARROW_COMPUTE #include "arrow/util/logging.h" -#include // -AL- TEMP remove later - namespace arrow { namespace compute { class FunctionRegistry::FunctionRegistryImpl { public: explicit FunctionRegistryImpl(FunctionRegistryImpl* parent = NULLPTR) - : parent_(parent) {// -AL- TEMP remove later - std::cout << "-AL- FunctionRegistryImpl constructor called\n"; - } - ~FunctionRegistryImpl() { // -AL- TEMP remove later - size_t name_size = name_to_function_.size(); - std::cout << "-AL- ~FunctionRegistryImpl name_to_function_ size:" << name_size << - "\n"; - - //auto it = name_to_function_.find(std::string("split_pattern")); - //auto func = it->second; - //for (auto& kv : name_to_function_) { - // kv.second.reset(); - //} - //name_to_function_.clear(); - - std::cout << "-AL- ~FunctionRegistryImpl destructor called\n"; - } + : parent_(parent) {} + ~FunctionRegistryImpl() {} Status CanAddFunction(std::shared_ptr function, bool allow_overwrite) { if (parent_ != NULLPTR) { @@ -144,8 +127,7 @@ class FunctionRegistry::FunctionRegistryImpl { const Function* cast_function() { return cast_function_; } - void ClearFunctioRegistry() { name_to_function_.clear(); - } + void ClearFunctioRegistry() { name_to_function_.clear(); } private: // must not acquire mutex @@ -235,7 +217,6 @@ class FunctionRegistry::FunctionRegistryImpl { }; std::unique_ptr FunctionRegistry::Make() { - std::cout << "-AL- Make is called once TEMP \n"; return std::unique_ptr(new FunctionRegistry()); } @@ -298,14 +279,11 @@ int FunctionRegistry::num_functions() const { return impl_->num_functions(); } const Function* FunctionRegistry::cast_function() const { return impl_->cast_function(); } -void FunctionRegistry::ClearFunctioRegistry() { - impl_->ClearFunctioRegistry(); -} +void FunctionRegistry::ClearFunctioRegistry() { impl_->ClearFunctioRegistry(); } namespace internal { static std::unique_ptr CreateBuiltInRegistry() { - std::cout << "-AL- CreateBuiltInRegistry is called once TEMP \n"; auto registry = FunctionRegistry::Make(); // Register core kernels diff --git a/cpp/src/arrow/compute/registry.h b/cpp/src/arrow/compute/registry.h index 6ea8bb94f80..e3b65fd06a1 100644 --- a/cpp/src/arrow/compute/registry.h +++ b/cpp/src/arrow/compute/registry.h @@ -111,8 +111,10 @@ class ARROW_EXPORT FunctionRegistry { /// /// Helpful for get cast function as needed. const Function* cast_function() const; - - // -AL- todo docs? + + /// \brief Clear function registry + /// + /// Helpful to avoid segfault from race condition. Call this function before DLL unload. void ClearFunctioRegistry(); private: diff --git a/cpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.cc b/cpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.cc index 17fcea154a1..f412deaf2db 100644 --- a/cpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.cc +++ b/cpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.cc @@ -29,11 +29,10 @@ namespace arrow::flight::sql::odbc { -// -AL- todo rename later -auto alinatest_env = ::testing::AddGlobalTestEnvironment(new AlinaTestEnvironment); +auto odbc_util_env = ::testing::AddGlobalTestEnvironment(new OdbcUtilEnvironment); -AlinaTestEnvironment* GetAlinaTestEnv() { - return ::arrow::internal::checked_cast(alinatest_env); +OdbcUtilEnvironment* GetOdbcUtilEnv() { + return ::arrow::internal::checked_cast(odbc_util_env); } void ODBCRemoteTestBase::AllocEnvConnHandles(SQLINTEGER odbc_ver) { @@ -63,15 +62,15 @@ void ODBCRemoteTestBase::ConnectWithString(std::string connect_str) { SQLSMALLINT out_str_len; // -AL- once local conn handles are removed,can rename back to conn - SQLHDBC global_conn = GetAlinaTestEnv()->getConnHandle(); - //SQLHDBC global_conn = conn; //-AL- TEMP + SQLHDBC global_conn = GetOdbcUtilEnv()->getConnHandle(); + // SQLHDBC global_conn = conn; //-AL- TEMP // Connecting to ODBC server. ASSERT_EQ(SQL_SUCCESS, SQLDriverConnect(global_conn, NULL, &connect_str0[0], static_cast(connect_str0.size()), out_str, kOdbcBufferSize, &out_str_len, SQL_DRIVER_NOPROMPT)) - << GetOdbcErrorMessage(SQL_HANDLE_DBC, global_conn); + << GetOdbcErrorMessage(SQL_HANDLE_DBC, global_conn); // GH-47710: TODO Allocate a statement using alloc handle // ASSERT_EQ(SQL_SUCCESS, SQLAllocHandle(SQL_HANDLE_STMT, conn, &stmt)); @@ -81,8 +80,8 @@ void ODBCRemoteTestBase::Disconnect() { // GH-47710: TODO Close statement // EXPECT_EQ(SQL_SUCCESS, SQLFreeHandle(SQL_HANDLE_STMT, stmt)); - // -AL- once local conn handles are removed,can rename back to conn - SQLHDBC global_conn = GetAlinaTestEnv()->getConnHandle(); + // -AL- once local conn handles are removed,can rename back to conn + SQLHDBC global_conn = GetOdbcUtilEnv()->getConnHandle(); // Disconnect from ODBC EXPECT_EQ(SQL_SUCCESS, SQLDisconnect(global_conn)) diff --git a/cpp/src/arrow/flight/sql/odbc/tests/odbc_test_util.h b/cpp/src/arrow/flight/sql/odbc/tests/odbc_test_util.h index 4ffbdb7159d..ca8ba1848d8 100644 --- a/cpp/src/arrow/flight/sql/odbc/tests/odbc_test_util.h +++ b/cpp/src/arrow/flight/sql/odbc/tests/odbc_test_util.h @@ -19,7 +19,7 @@ #include "arrow/flight/sql/odbc/odbc_impl/platform.h" -#include "arrow/compute/api.h" // -AL- attempt to keep registry alive +#include "arrow/compute/api.h" #include #include @@ -29,10 +29,9 @@ namespace arrow::flight::sql::odbc { // -AL- todo update to actual ODBC allocation -class AlinaTestEnvironment : public ::testing::Environment { +class OdbcUtilEnvironment : public ::testing::Environment { public: void SetUp() override { - int x = 1; // -AL- todo add env_v2 for v2 after global setup/teardown works. // Allocate an environment handle @@ -45,27 +44,20 @@ class AlinaTestEnvironment : public ::testing::Environment { // Allocate a connection using alloc handle ASSERT_EQ(SQL_SUCCESS, SQLAllocHandle(SQL_HANDLE_DBC, env, &conn)); - std::cout << "-AL- AlinaTestEnvironment::SetUp\n"; + std::cout << "-AL- OdbcUtilEnvironment::SetUp\n"; } void TearDown() override { - - - // -AL- this doesn't work - //// Remove function registry before test exits + // Clear function registry before test exits auto reg = arrow::compute::GetFunctionRegistry(); reg->ClearFunctioRegistry(); - // delete reg; - - int y = 1; // Free connection handle EXPECT_EQ(SQL_SUCCESS, SQLFreeHandle(SQL_HANDLE_DBC, conn)); // Free environment handle EXPECT_EQ(SQL_SUCCESS, SQLFreeHandle(SQL_HANDLE_ENV, env)); - std::cout << "-AL- AlinaTestEnvironment::TearDown\n"; - + std::cout << "-AL- OdbcUtilEnvironment::TearDown\n"; } SQLHENV getEnvHandle() { return env; }