diff --git a/.github/workflows/cpp_windows.yml b/.github/workflows/cpp_windows.yml index cc48f277104..5215c97f9ca 100644 --- a/.github/workflows/cpp_windows.yml +++ b/.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" 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" \ 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 diff --git a/cpp/src/arrow/compute/registry.cc b/cpp/src/arrow/compute/registry.cc index be0ebd32016..9ada51611f4 100644 --- a/cpp/src/arrow/compute/registry.cc +++ b/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 CreateBuiltInRegistry() { diff --git a/cpp/src/arrow/compute/registry.h b/cpp/src/arrow/compute/registry.h index f31c4c1ba59..e3b65fd06a1 100644 --- a/cpp/src/arrow/compute/registry.h +++ b/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(); 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) 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 { 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/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/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 ead2beada4b..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 @@ -36,6 +37,7 @@ #include #include #include +#include #include using ODBC::ODBCConnection; @@ -531,7 +533,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/cpp/src/arrow/flight/sql/odbc/tests/CMakeLists.txt b/cpp/src/arrow/flight/sql/odbc/tests/CMakeLists.txt index 4bc240637e7..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 @@ -44,3 +45,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) 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..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 @@ -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,12 @@ namespace arrow::flight::sql::odbc { +auto odbc_util_env = ::testing::AddGlobalTestEnvironment(new OdbcUtilEnvironment); + +OdbcUtilEnvironment* GetOdbcUtilEnv() { + return ::arrow::internal::checked_cast(odbc_util_env); +} + void ODBCRemoteTestBase::AllocEnvConnHandles(SQLINTEGER odbc_ver) { // Allocate an environment handle ASSERT_EQ(SQL_SUCCESS, SQLAllocEnv(&env)); @@ -54,12 +61,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 = GetOdbcUtilEnv()->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 +80,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 = GetOdbcUtilEnv()->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_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" 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..ca8ba1848d8 --- /dev/null +++ b/cpp/src/arrow/flight/sql/odbc/tests/odbc_test_util.h @@ -0,0 +1,75 @@ +// 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" + +#include +#include + +#include + +namespace arrow::flight::sql::odbc { + +// -AL- todo update to actual ODBC allocation +class OdbcUtilEnvironment : public ::testing::Environment { + public: + void SetUp() override { + // -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- OdbcUtilEnvironment::SetUp\n"; + } + + void TearDown() override { + // Clear function registry before test exits + auto reg = arrow::compute::GetFunctionRegistry(); + reg->ClearFunctioRegistry(); + + // 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- OdbcUtilEnvironment::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 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..244ce5f080a --- /dev/null +++ b/cpp/src/arrow/flight/sql/sql_info_undef.h @@ -0,0 +1,172 @@ +// 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 + +#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..14a323828b5 100644 --- a/cpp/src/arrow/flight/sql/types.h +++ b/cpp/src/arrow/flight/sql/types.h @@ -25,6 +25,7 @@ #include #include +#include "arrow/flight/sql/sql_info_undef.h" #include "arrow/flight/sql/visibility.h" #include "arrow/type_fwd.h" diff --git a/testing b/testing index 9a56b4747da..8da05fdd62a 160000 --- a/testing +++ b/testing @@ -1 +1 @@ -Subproject commit 9a56b4747da5539e41b25a4d4827d0ed801ecd30 +Subproject commit 8da05fdd62a7243ef77aa9757acb62e0586a4d0c