GH-48019 : [C++][FlightRPC] work-in-progress Enable ODBC in MSVC CI

.github/workflows/cpp_windows.yml

@@ -36,15 +36,18 @@ on:
 jobs:
   windows:
     runs-on: ${{ inputs.os }}
-    timeout-minutes: 60
+    timeout-minutes: 240
     env:
-      ARROW_BOOST_USE_SHARED: OFF
       ARROW_BUILD_BENCHMARKS: ON
       ARROW_BUILD_SHARED: ON
-      ARROW_BUILD_STATIC: OFF
+      ARROW_BUILD_STATIC: ON
       ARROW_BUILD_TESTS: ON
+      ARROW_BUILD_TYPE: release
       ARROW_DATASET: ON
-      ARROW_FLIGHT: OFF
+      ARROW_DEPENDENCY_SOURCE: VCPKG
+      ARROW_FLIGHT: ON
+      ARROW_FLIGHT_SQL: ON
+      ARROW_FLIGHT_SQL_ODBC: ON
       ARROW_HDFS: ON
       ARROW_HOME: /usr
       ARROW_JEMALLOC: OFF
@@ -62,11 +65,12 @@ 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
       CMAKE_UNITY_BUILD: ON
+      VCPKG_BINARY_SOURCES: 'clear;nuget,GitHub,readwrite'
+      VCPKG_DEFAULT_TRIPLET: x64-windows
     steps:
       - name: Disable Crash Dialogs
         run: |
@@ -110,15 +114,53 @@ 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
+        run: |
+          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: |
+          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
+
+          # 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"

ci/scripts/cpp_test.sh

@@ -144,8 +144,9 @@ if [ "${ARROW_USE_MESON:-OFF}" = "OFF" ] && \
       CMAKE_PREFIX_PATH+="/lib/cmake/"
       ;;
   esac
+  # Search vcpkg before <prefix>/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" \

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

cpp/src/arrow/compute/registry.cc

@@ -127,6 +127,8 @@ 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) {
@@ -277,6 +279,8 @@ 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<FunctionRegistry> CreateBuiltInRegistry() {

cpp/src/arrow/compute/registry.h

@@ -112,6 +112,11 @@ class ARROW_EXPORT FunctionRegistry {
   /// Helpful for get cast function as needed.
   const Function* cast_function() const;
 
+  /// \brief Clear function registry
+  ///
+  /// Helpful to avoid segfault from race condition. Call this function before DLL unload.
+  void ClearFunctioRegistry();
+
  private:
   FunctionRegistry();
 

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)

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 {

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

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

@@ -18,9 +18,9 @@
 #pragma once
 
 #include <atomic>
-#include <boost/optional.hpp>
 #include <condition_variable>
 #include <mutex>
+#include <optional>
 #include <thread>
 #include <vector>
 
@@ -43,7 +43,7 @@ class BlockingQueue {
   std::atomic<bool> closed_{false};
 
  public:
-  typedef std::function<boost::optional<T>(void)> Supplier;
+  typedef std::function<std::optional<T>(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);
       }

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 <optional>
 #include <utility>
 
 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<Connection::Attribute>& login_timeout =
+    const std::optional<Connection::Attribute>& login_timeout =
         connection.GetAttribute(Connection::LOGIN_TIMEOUT);
     if (login_timeout && boost::get<uint32_t>(*login_timeout) > 0) {
       // ODBC's LOGIN_TIMEOUT attribute and FlightCallOptions.timeout use

cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_connection.cc

@@ -31,7 +31,6 @@
 #include <boost/algorithm/string/join.hpp>
 #include <boost/asio/ip/address.hpp>
 #include <boost/lexical_cast.hpp>
-#include <boost/optional.hpp>
 #include "arrow/flight/sql/odbc/odbc_impl/exceptions.h"
 
 #include <sql.h>
@@ -199,7 +198,7 @@ void FlightSqlConnection::PopulateMetadataSettings(
   metadata_settings_.chunk_buffer_capacity = GetChunkBufferCapacity(conn_property_map);
 }
 
-boost::optional<int32_t> FlightSqlConnection::GetStringColumnLength(
+std::optional<int32_t> FlightSqlConnection::GetStringColumnLength(
     const Connection::ConnPropertyMap& conn_property_map) {
   const int32_t min_string_column_length = 1;
 
@@ -214,7 +213,7 @@ boost::optional<int32_t> 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::Attribute>& connection_timeout =
+  const std::optional<Connection::Attribute>& connection_timeout =
       closed_ ? GetAttribute(LOGIN_TIMEOUT) : GetAttribute(CONNECTION_TIMEOUT);
   if (connection_timeout && boost::get<uint32_t>(*connection_timeout) > 0) {
     call_options_.timeout =
@@ -388,17 +387,21 @@ bool FlightSqlConnection::SetAttribute(Connection::AttributeId attribute,
   }
 }
 
-boost::optional<Connection::Attribute> FlightSqlConnection::GetAttribute(
+std::optional<Connection::Attribute> FlightSqlConnection::GetAttribute(
     Connection::AttributeId attribute) {
   switch (attribute) {
     case ACCESS_MODE:
       // FlightSQL does not provide this metadata.
-      return boost::make_optional(Attribute(static_cast<uint32_t>(SQL_MODE_READ_WRITE)));
+      return std::make_optional(Attribute(static_cast<uint32_t>(SQL_MODE_READ_WRITE)));
     case PACKET_SIZE:
-      return boost::make_optional(Attribute(static_cast<uint32_t>(0)));
+      return std::make_optional(Attribute(static_cast<uint32_t>(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;
+      }
   }
 }
 

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 <optional>
 #include <vector>
 #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<Connection::Attribute> GetAttribute(
+  std::optional<Connection::Attribute> 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<int32_t> GetStringColumnLength(
-      const ConnPropertyMap& conn_property_map);
+  std::optional<int32_t> GetStringColumnLength(const ConnPropertyMap& conn_property_map);
 
   bool GetUseWideChar(const ConnPropertyMap& conn_property_map);
 

cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_connection_test.cc

@@ -21,14 +21,16 @@
 #include "arrow/flight/types.h"
 #include "gtest/gtest.h"
 
+#include <optional>
+
 namespace arrow::flight::sql::odbc {
 
 TEST(AttributeTests, SetAndGetAttribute) {
   FlightSqlConnection connection(OdbcVersion::V_3);
   connection.SetClosed(false);
 
   connection.SetAttribute(Connection::CONNECTION_TIMEOUT, static_cast<uint32_t>(200));
-  const boost::optional<Connection::Attribute> first_value =
+  const std::optional<Connection::Attribute> 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<uint32_t>(300));
 
-  const boost::optional<Connection::Attribute> change_value =
+  const std::optional<Connection::Attribute> 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<Connection::Attribute> optional =
+  const std::optional<Connection::Attribute> 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<int32_t> actual_string_column_length =
+  const std::optional<int32_t> actual_string_column_length =
       connection.GetStringColumnLength(properties);
 
   EXPECT_TRUE(actual_string_column_length);

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 <boost/optional.hpp>
+#include <optional>
 #include <utility>
 #include "arrow/flight/sql/odbc/odbc_impl/exceptions.h"
 
@@ -89,13 +89,17 @@ bool FlightSqlStatement::SetAttribute(StatementAttributeId attribute,
   }
 }
 
-boost::optional<Statement::Attribute> FlightSqlStatement::GetAttribute(
+std::optional<Statement::Attribute> 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<std::shared_ptr<ResultSetMetadata>> FlightSqlStatement::Prepare(
+std::optional<std::shared_ptr<ResultSetMetadata>> FlightSqlStatement::Prepare(
     const std::string& query) {
   ClosePreparedStatementIfAny(prepared_statement_);
 
@@ -107,7 +111,7 @@ boost::optional<std::shared_ptr<ResultSetMetadata>> FlightSqlStatement::Prepare(
 
   const auto& result_set_metadata = std::make_shared<FlightSqlResultSetMetadata>(
       prepared_statement_->dataset_schema(), metadata_settings_);
-  return boost::optional<std::shared_ptr<ResultSetMetadata>>(result_set_metadata);
+  return std::optional<std::shared_ptr<ResultSetMetadata>>(result_set_metadata);
 }
 
 bool FlightSqlStatement::ExecutePrepared() {