diff --git a/.github/workflows/cpp_extra.yml b/.github/workflows/cpp_extra.yml index b38ccaa2779..b1971d790c9 100644 --- a/.github/workflows/cpp_extra.yml +++ b/.github/workflows/cpp_extra.yml @@ -357,6 +357,10 @@ jobs: ARROW_BUILD_TESTS: ON ARROW_FLIGHT_SQL_ODBC: ON ARROW_HOME: /tmp/local + ARROW_DEPENDENCY_USE_SHARED: OFF + ARROW_DEPENDENCY_SOURCE: BUNDLED + ARROW_MIMALLOC: OFF + CMAKE_CXX_STANDARD: "20" steps: - name: Checkout Arrow uses: actions/checkout@v6.0.1 @@ -366,6 +370,22 @@ jobs: - name: Install Dependencies run: | brew bundle --file=cpp/Brewfile + + # We want to use bundled RE2 for static linking. If + # Homebrew's RE2 is installed, its header file may be used. + # We uninstall Homebrew's RE2 to ensure using bundled RE2. + brew uninstall grpc || : # gRPC depends on RE2 + brew uninstall grpc@1.54 || : # gRPC 1.54 may be installed too + brew uninstall re2 + + # We want to use bundled Protobuf for static linking. If + # Homebrew's Protobuf is installed, its library file may be + # used on test We uninstall Homebrew's Protobuf to ensure using + # bundled Protobuf. + brew uninstall protobuf + + # We want to use bundled Boost for static linking. + brew uninstall boost - name: Setup ccache run: | ci/scripts/ccache_setup.sh @@ -430,6 +450,7 @@ jobs: CMAKE_INSTALL_PREFIX: /usr VCPKG_BINARY_SOURCES: 'clear;nugettimeout,600;nuget,GitHub,readwrite' VCPKG_DEFAULT_TRIPLET: x64-windows + CMAKE_CXX_STANDARD: "20" steps: - name: Disable Crash Dialogs run: | diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index e84b2accb8b..b303463a0bf 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -1114,6 +1114,22 @@ function(build_boost) else() list(APPEND BOOST_EXCLUDE_LIBRARIES uuid) endif() + if(ARROW_FLIGHT_SQL_ODBC) + # GH-49244: Replace boost beast with alternatives in ODBC + # GH-49243: Replace boost variant with std::variant in ODBC + # GH-49245: Replace boost xpressive with alternatives in ODBC + list(APPEND + BOOST_INCLUDE_LIBRARIES + beast + variant + xpressive) + else() + list(APPEND + BOOST_EXCLUDE_LIBRARIES + beast + variant + xpressive) + endif() set(BOOST_SKIP_INSTALL_RULES ON) if(NOT ARROW_ENABLE_THREADING) set(BOOST_UUID_LINK_LIBATOMIC OFF) @@ -3023,8 +3039,8 @@ function(build_cares) if(APPLE) # libresolv must be linked from c-ares version 1.16.1 find_library(LIBRESOLV_LIBRARY NAMES resolv libresolv REQUIRED) - set_target_properties(c-ares::cares PROPERTIES INTERFACE_LINK_LIBRARIES - "${LIBRESOLV_LIBRARY}") + set_target_properties(c-ares PROPERTIES INTERFACE_LINK_LIBRARIES + "${LIBRESOLV_LIBRARY}") endif() set(ARROW_BUNDLED_STATIC_LIBS diff --git a/cpp/src/arrow/flight/sql/odbc/CMakeLists.txt b/cpp/src/arrow/flight/sql/odbc/CMakeLists.txt index 39040c45024..80d3a67c293 100644 --- a/cpp/src/arrow/flight/sql/odbc/CMakeLists.txt +++ b/cpp/src/arrow/flight/sql/odbc/CMakeLists.txt @@ -19,6 +19,11 @@ # GH-44792: Arrow will switch to C++ 20 set(CMAKE_CXX_STANDARD 20) +if(APPLE) + # CMAKE_FIND_LIBRARY_SUFFIXES. + set(APPEND CMAKE_FIND_LIBRARY_SUFFIXES ".a") +endif() + if(WIN32) if(MSVC_VERSION GREATER_EQUAL 1900) set(ODBCINST legacy_stdio_definitions odbccp32 shlwapi) @@ -61,33 +66,56 @@ if(WIN32) list(APPEND ARROW_FLIGHT_SQL_ODBC_SRCS odbc.def install/versioninfo.rc) endif() -add_arrow_lib(arrow_flight_sql_odbc - CMAKE_PACKAGE_NAME - ArrowFlightSqlOdbc - PKG_CONFIG_NAME - arrow-flight-sql-odbc - OUTPUTS - ARROW_FLIGHT_SQL_ODBC_LIBRARIES - SOURCES - ${ARROW_FLIGHT_SQL_ODBC_SRCS} - DEPENDENCIES - arrow_flight_sql - DEFINITIONS - UNICODE - SHARED_LINK_FLAGS - ${ARROW_VERSION_SCRIPT_FLAGS} # Defined in cpp/arrow/CMakeLists.txt - SHARED_LINK_LIBS - arrow_flight_sql_shared - SHARED_INSTALL_INTERFACE_LIBS - ArrowFlight::arrow_flight_sql_shared - STATIC_LINK_LIBS - arrow_flight_sql_static - STATIC_INSTALL_INTERFACE_LIBS - ArrowFlight::arrow_flight_sql_static - SHARED_PRIVATE_LINK_LIBS - ODBC::ODBC - ${ODBCINST} - arrow_odbc_spi_impl) +# On Windows, dynmaic build for ODBC is supported. +# On unix systems, static build for ODBC is supported, all libraries are linked statically on unix. +if(WIN32) + add_arrow_lib(arrow_flight_sql_odbc + CMAKE_PACKAGE_NAME + ArrowFlightSqlOdbc + PKG_CONFIG_NAME + arrow-flight-sql-odbc + OUTPUTS + ARROW_FLIGHT_SQL_ODBC_LIBRARIES + SOURCES + ${ARROW_FLIGHT_SQL_ODBC_SRCS} + DEFINITIONS + UNICODE + SHARED_LINK_FLAGS + ${ARROW_VERSION_SCRIPT_FLAGS} # Defined in cpp/arrow/CMakeLists.txt + SHARED_LINK_LIBS + arrow_flight_sql_shared + arrow_odbc_spi_impl + SHARED_INSTALL_INTERFACE_LIBS + ArrowFlight::arrow_flight_sql_shared + STATIC_LINK_LIBS + arrow_flight_sql_static + STATIC_INSTALL_INTERFACE_LIBS + ArrowFlight::arrow_flight_sql_static + SHARED_PRIVATE_LINK_LIBS + ODBC::ODBC + ${ODBCINST}) +else() + # Unix + add_arrow_lib(arrow_flight_sql_odbc + CMAKE_PACKAGE_NAME + ArrowFlightSqlOdbc + PKG_CONFIG_NAME + arrow-flight-sql-odbc + OUTPUTS + ARROW_FLIGHT_SQL_ODBC_LIBRARIES + SOURCES + ${ARROW_FLIGHT_SQL_ODBC_SRCS} + DEFINITIONS + UNICODE + SHARED_LINK_FLAGS + ${ARROW_VERSION_SCRIPT_FLAGS} # Defined in cpp/arrow/CMakeLists.txt + STATIC_LINK_LIBS + iodbc + ODBC::ODBC + ${ODBCINST} + SHARED_LINK_LIBS + arrow_odbc_spi_impl) +endif() foreach(LIB_TARGET ${ARROW_FLIGHT_SQL_ODBC_LIBRARIES}) target_compile_definitions(${LIB_TARGET} PRIVATE ARROW_FLIGHT_SQL_ODBC_EXPORTING) 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 e58558258df..0897248f6ce 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/CMakeLists.txt +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/CMakeLists.txt @@ -15,7 +15,7 @@ # specific language governing permissions and limitations # under the License. -add_library(arrow_odbc_spi_impl +add_library(arrow_odbc_spi_impl STATIC accessors/binary_array_accessor.cc accessors/binary_array_accessor.h accessors/boolean_array_accessor.cc @@ -133,8 +133,11 @@ endif() if(APPLE) target_include_directories(arrow_odbc_spi_impl SYSTEM BEFORE PUBLIC ${ODBC_INCLUDE_DIR}) target_link_libraries(arrow_odbc_spi_impl - PUBLIC arrow_flight_sql_shared arrow_compute_shared Boost::locale - iodbc) + PUBLIC arrow_flight_sql_static + arrow_compute_static + Boost::locale + Boost::headers + RapidJSON) else() find_package(ODBC REQUIRED) target_include_directories(arrow_odbc_spi_impl PUBLIC ${ODBC_INCLUDE_DIR}) @@ -143,14 +146,6 @@ else() ${ODBCINST}) endif() -set_target_properties(arrow_odbc_spi_impl - PROPERTIES ARCHIVE_OUTPUT_DIRECTORY - ${CMAKE_BINARY_DIR}/$/lib - LIBRARY_OUTPUT_DIRECTORY - ${CMAKE_BINARY_DIR}/$/lib - RUNTIME_OUTPUT_DIRECTORY - ${CMAKE_BINARY_DIR}/$/lib) - # CLI add_executable(arrow_odbc_spi_impl_cli main.cc) set_target_properties(arrow_odbc_spi_impl_cli @@ -159,6 +154,16 @@ set_target_properties(arrow_odbc_spi_impl_cli target_link_libraries(arrow_odbc_spi_impl_cli arrow_odbc_spi_impl) # Unit tests + +# On Windows, dynamic linking ODBC is supported. +# On unix systems, static linking ODBC is supported, thus the library linking is static. +if(WIN32) + set(ODBC_SPI_IMPL_TEST_LINK_LIBS arrow_flight_testing_shared) +else() + # unix + set(ODBC_SPI_IMPL_TEST_LINK_LIBS arrow_flight_testing_static) +endif() + add_arrow_test(odbc_spi_impl_test SOURCES accessors/boolean_array_accessor_test.cc @@ -177,4 +182,4 @@ add_arrow_test(odbc_spi_impl_test util_test.cc EXTRA_LINK_LIBS arrow_odbc_spi_impl - arrow_flight_testing_shared) + ${ODBC_SPI_IMPL_TEST_LINK_LIBS}) diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/config/connection_string_parser.h b/cpp/src/arrow/flight/sql/odbc/odbc_impl/config/connection_string_parser.h index dd937410adc..2e7196e322f 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/config/connection_string_parser.h +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/config/connection_string_parser.h @@ -19,7 +19,7 @@ #include -#include "config/configuration.h" +#include "arrow/flight/sql/odbc/odbc_impl/config/configuration.h" namespace arrow::flight::sql::odbc { namespace config { diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/ui/add_property_window.cc b/cpp/src/arrow/flight/sql/odbc/odbc_impl/ui/add_property_window.cc index 634fd77862e..6518e48459e 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/ui/add_property_window.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/ui/add_property_window.cc @@ -15,7 +15,7 @@ // specific language governing permissions and limitations // under the License. -#include "ui/add_property_window.h" +#include "arrow/flight/sql/odbc/odbc_impl/ui/add_property_window.h" #include @@ -25,8 +25,8 @@ #include #include "arrow/flight/sql/odbc/odbc_impl/exceptions.h" -#include "ui/custom_window.h" -#include "ui/window.h" +#include "arrow/flight/sql/odbc/odbc_impl/ui/custom_window.h" +#include "arrow/flight/sql/odbc/odbc_impl/ui/window.h" namespace arrow::flight::sql::odbc { namespace config { diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/ui/custom_window.cc b/cpp/src/arrow/flight/sql/odbc/odbc_impl/ui/custom_window.cc index 179303b68e3..47149a33a56 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/ui/custom_window.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/ui/custom_window.cc @@ -28,7 +28,7 @@ #include #include "arrow/flight/sql/odbc/odbc_impl/exceptions.h" -#include "ui/custom_window.h" +#include "arrow/flight/sql/odbc/odbc_impl/ui/custom_window.h" namespace arrow::flight::sql::odbc { namespace config { diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/ui/custom_window.h b/cpp/src/arrow/flight/sql/odbc/odbc_impl/ui/custom_window.h index a6580011485..9745f58b2e2 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/ui/custom_window.h +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/ui/custom_window.h @@ -17,7 +17,7 @@ #pragma once -#include "ui/window.h" +#include "arrow/flight/sql/odbc/odbc_impl/ui/window.h" namespace arrow::flight::sql::odbc { namespace config { diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/ui/dsn_configuration_window.h b/cpp/src/arrow/flight/sql/odbc/odbc_impl/ui/dsn_configuration_window.h index bfac68df8b9..74a75053736 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/ui/dsn_configuration_window.h +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/ui/dsn_configuration_window.h @@ -17,8 +17,8 @@ #pragma once -#include "config/configuration.h" -#include "ui/custom_window.h" +#include "arrow/flight/sql/odbc/odbc_impl/config/configuration.h" +#include "arrow/flight/sql/odbc/odbc_impl/ui/custom_window.h" namespace arrow::flight::sql::odbc { namespace config { diff --git a/cpp/src/arrow/flight/sql/odbc/tests/CMakeLists.txt b/cpp/src/arrow/flight/sql/odbc/tests/CMakeLists.txt index ef0c7271ec2..b44846fb350 100644 --- a/cpp/src/arrow/flight/sql/odbc/tests/CMakeLists.txt +++ b/cpp/src/arrow/flight/sql/odbc/tests/CMakeLists.txt @@ -25,29 +25,60 @@ set(ARROW_FLIGHT_SQL_MOCK_SERVER_SRCS ../../example/sqlite_server.cc ../../example/sqlite_tables_schema_batch_reader.cc) -add_arrow_test(flight_sql_odbc_test - SOURCES - odbc_test_suite.cc - odbc_test_suite.h - columns_test.cc - connection_attr_test.cc - connection_info_test.cc - connection_test.cc - errors_test.cc - get_functions_test.cc - statement_attr_test.cc - statement_test.cc - tables_test.cc - type_info_test.cc - # Enable Protobuf cleanup after test execution - # GH-46889: move protobuf_test_util to a more common location - ../../../../engine/substrait/protobuf_test_util.cc - ${ARROW_FLIGHT_SQL_MOCK_SERVER_SRCS} - EXTRA_LINK_LIBS - ${ODBC_LIBRARIES} - ${ODBCINST} - ${SQLite3_LIBRARIES} - arrow_odbc_spi_impl) +if(ARROW_TEST_LINKAGE STREQUAL "static") + set(ARROW_FLIGHT_SQL_ODBC_TEST_LINK_LIBS arrow_flight_sql_odbc_static + ${ARROW_TEST_STATIC_LINK_LIBS}) +else() + set(ARROW_FLIGHT_SQL_ODBC_TEST_LINK_LIBS arrow_flight_sql_odbc_shared + ${ARROW_TEST_SHARED_LINK_LIBS}) +endif() + +set(ARROW_FLIGHT_SQL_ODBC_TEST_SRCS + odbc_test_suite.cc + odbc_test_suite.h + columns_test.cc + connection_attr_test.cc + connection_info_test.cc + connection_test.cc + errors_test.cc + get_functions_test.cc + statement_attr_test.cc + statement_test.cc + tables_test.cc + type_info_test.cc + # Enable Protobuf cleanup after test execution + # GH-46889: move protobuf_test_util to a more common location + ../../../../engine/substrait/protobuf_test_util.cc) + +# On Windows, dynamic linking ODBC is supported, tests link libraries dynamically. +# On unix systems, static linking ODBC is supported, thus tests link libraries statically. +if(WIN32) + add_arrow_test(flight_sql_odbc_test + SOURCES + ${ARROW_FLIGHT_SQL_ODBC_TEST_SRCS} + ${ARROW_FLIGHT_SQL_MOCK_SERVER_SRCS} + DEFINITIONS + UNICODE + EXTRA_LINK_LIBS + ${ODBC_LIBRARIES} + ${ODBCINST} + ${SQLite3_LIBRARIES} + ${ARROW_FLIGHT_SQL_ODBC_TEST_LINK_LIBS}) +elseif(APPLE) + # macOS + add_arrow_test(flight_sql_odbc_test + SOURCES + ${ARROW_FLIGHT_SQL_ODBC_TEST_SRCS} + ${ARROW_FLIGHT_SQL_MOCK_SERVER_SRCS} + DEFINITIONS + UNICODE + STATIC_LINK_LIBS + iodbc + ${ODBC_LIBRARIES} + ${ODBCINST} + ${SQLite3_LIBRARIES} + ${ARROW_FLIGHT_SQL_ODBC_TEST_LINK_LIBS}) +endif() find_package(ODBC REQUIRED) target_link_libraries(arrow-flight-sql-odbc-test PRIVATE ODBC::ODBC) diff --git a/cpp/src/arrow/flight/sql/odbc/tests/errors_test.cc b/cpp/src/arrow/flight/sql/odbc/tests/errors_test.cc index 34a32738455..36f91f827e4 100644 --- a/cpp/src/arrow/flight/sql/odbc/tests/errors_test.cc +++ b/cpp/src/arrow/flight/sql/odbc/tests/errors_test.cc @@ -507,7 +507,7 @@ TYPED_TEST(ErrorsOdbcV2Test, TestSQLErrorStmtErrorODBCVer2) { // When application passes buffer length greater than SQL_MAX_MESSAGE_LENGTH (512), // DM passes 512 as buffer length to SQLError. - std::wstring wsql = L"1"; + std::wstring wsql = L"SELECT * from non_existent_table;"; std::vector sql0(wsql.begin(), wsql.end()); ASSERT_EQ(SQL_ERROR, @@ -515,11 +515,11 @@ TYPED_TEST(ErrorsOdbcV2Test, TestSQLErrorStmtErrorODBCVer2) { SQLWCHAR sql_state[6] = {0}; SQLINTEGER native_error = 0; - SQLWCHAR message[SQL_MAX_MESSAGE_LENGTH] = {0}; SQLSMALLINT message_length = 0; - ASSERT_EQ(SQL_SUCCESS, SQLError(nullptr, nullptr, this->stmt, sql_state, &native_error, - message, SQL_MAX_MESSAGE_LENGTH, &message_length)); - + SQLWCHAR message[SQL_MAX_MESSAGE_LENGTH] = {0}; + ASSERT_EQ(SQL_SUCCESS, + SQLError(SQL_NULL_HENV, this->conn, this->stmt, sql_state, &native_error, + message, SQL_MAX_MESSAGE_LENGTH, &message_length)); EXPECT_GT(message_length, 70); EXPECT_EQ(100, native_error); 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 3115cd62754..419f698c5d9 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 @@ -236,6 +236,7 @@ static constexpr std::string_view kErrorStateHY106 = "HY106"; static constexpr std::string_view kErrorStateHY114 = "HY114"; static constexpr std::string_view kErrorStateHY118 = "HY118"; static constexpr std::string_view kErrorStateHYC00 = "HYC00"; +static constexpr std::string_view kErrorStateIM001 = "IM001"; static constexpr std::string_view kErrorStateS1002 = "S1002"; static constexpr std::string_view kErrorStateS1004 = "S1004"; static constexpr std::string_view kErrorStateS1010 = "S1010"; diff --git a/cpp/src/arrow/flight/sql/odbc/tests/statement_attr_test.cc b/cpp/src/arrow/flight/sql/odbc/tests/statement_attr_test.cc index 0a4e99d33a6..c1b021bca65 100644 --- a/cpp/src/arrow/flight/sql/odbc/tests/statement_attr_test.cc +++ b/cpp/src/arrow/flight/sql/odbc/tests/statement_attr_test.cc @@ -477,15 +477,25 @@ TYPED_TEST(StatementAttributeTest, TestSQLSetStmtAttrFetchBookmarkPointer) { TYPED_TEST(StatementAttributeTest, TestSQLSetStmtAttrIMPParamDesc) { // Invalid use of an automatically allocated descriptor handle ValidateSetStmtAttrErrorCode(this->stmt, SQL_ATTR_IMP_PARAM_DESC, - static_cast(0), kErrorStateHY017); + static_cast(0), +#ifdef __APPLE__ + // static iODBC on MacOS returns IM001 for this case + kErrorStateIM001); +#else + kErrorStateHY017); +#endif // __APPLE__ } TYPED_TEST(StatementAttributeTest, TestSQLSetStmtAttrIMPRowDesc) { // Invalid use of an automatically allocated descriptor handle ValidateSetStmtAttrErrorCode(this->stmt, SQL_ATTR_IMP_ROW_DESC, static_cast(0), +#ifdef __APPLE__ + // static iODBC on MacOS returns IM001 for this case + kErrorStateIM001); +#else kErrorStateHY017); +#endif // __APPLE__ } - TYPED_TEST(StatementAttributeTest, TestSQLSetStmtAttrKeysetSizeUnsupported) { ValidateSetStmtAttr(this->stmt, SQL_ATTR_KEYSET_SIZE, static_cast(0)); }