diff --git a/.github/workflows/cpp_extra.yml b/.github/workflows/cpp_extra.yml index abe2aeb6877d..98ca4b317888 100644 --- a/.github/workflows/cpp_extra.yml +++ b/.github/workflows/cpp_extra.yml @@ -355,8 +355,11 @@ jobs: macos-version: "14" env: ARROW_BUILD_TESTS: ON + ARROW_DEPENDENCY_SOURCE: BUNDLED + ARROW_DEPENDENCY_USE_SHARED: OFF ARROW_FLIGHT_SQL_ODBC: ON ARROW_HOME: /tmp/local + ARROW_MIMALLOC: OFF steps: - name: Checkout Arrow uses: actions/checkout@v6.0.1 @@ -366,6 +369,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 @@ -395,6 +414,24 @@ jobs: export ARROW_CMAKE_ARGS="-DODBC_INCLUDE_DIR=$ODBC_INCLUDE_DIR" export CXXFLAGS="$CXXFLAGS -I$ODBC_INCLUDE_DIR" ci/scripts/cpp_build.sh $(pwd) $(pwd)/build + - name: Setup Python + uses: actions/setup-python@v6 + with: + python-version: 3 + - name: Setup Archery + run: python3 -m pip install -e dev/archery + - name: Check ODBC Dependency + run: | + # ODBC dependency should not include other Arrow or third party libraries + archery linking check-dependencies \ + --allow CoreFoundation \ + --allow libSystem \ + --allow libarrow_flight_sql_odbc \ + --allow libc++ \ + --allow libiconv \ + --allow libresolv \ + --allow libz \ + "$(pwd)/build/cpp/debug/libarrow_flight_sql_odbc.dylib" - name: Register Flight SQL ODBC Driver run: | sudo cpp/src/arrow/flight/sql/odbc/install/mac/install_odbc.sh $(pwd)/build/cpp/debug/libarrow_flight_sql_odbc.dylib diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index 935584c5349c..2da5c830d1e4 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) @@ -3025,8 +3041,7 @@ 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}") + target_link_libraries(c-ares INTERFACE ${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 39040c45024d..2560cdccd09e 100644 --- a/cpp/src/arrow/flight/sql/odbc/CMakeLists.txt +++ b/cpp/src/arrow/flight/sql/odbc/CMakeLists.txt @@ -15,10 +15,6 @@ # specific language governing permissions and limitations # under the License. -# Use C++ 20 for ODBC and its subdirectory -# GH-44792: Arrow will switch to C++ 20 -set(CMAKE_CXX_STANDARD 20) - if(WIN32) if(MSVC_VERSION GREATER_EQUAL 1900) set(ODBCINST legacy_stdio_definitions odbccp32 shlwapi) @@ -26,7 +22,7 @@ if(WIN32) set(ODBCINST odbccp32 shlwapi) endif() elseif(APPLE) - set(ODBCINST iodbcinst) + set(ODBCINST iodbc iodbcinst) else() set(ODBCINST odbcinst) endif() @@ -61,6 +57,27 @@ if(WIN32) list(APPEND ARROW_FLIGHT_SQL_ODBC_SRCS odbc.def install/versioninfo.rc) endif() +# On Windows, dynmaic build for ODBC is supported. +# On unix systems, static build for ODBC is supported, all libraries are linked statically on unix. +set(ARROW_FLIGHT_SQL_ODBC_DEPENDENCIES "") +set(ARROW_FLIGHT_SQL_ODBC_SHARED_INSTALL_INTERFACE_LIBS "") +set(ARROW_FLIGHT_SQL_ODBC_STATIC_INSTALL_INTERFACE_LIBS "") +set(ARROW_FLIGHT_SQL_ODBC_SHARED_PRIVATE_LINK_LIBS "") +if(WIN32) + set(ARROW_FLIGHT_SQL_ODBC_DEPENDENCIES arrow_flight_sql) + set(ARROW_FLIGHT_SQL_ODBC_SHARED_LINK_LIBS arrow_flight_sql_shared arrow_odbc_spi_impl) + set(ARROW_FLIGHT_SQL_ODBC_STATIC_LINK_LIBS arrow_flight_sql_static) + list(APPEND ARROW_FLIGHT_SQL_ODBC_SHARED_INSTALL_INTERFACE_LIBS + ArrowFlight::arrow_flight_sql_shared) + list(APPEND ARROW_FLIGHT_SQL_ODBC_STATIC_INSTALL_INTERFACE_LIBS + ArrowFlight::arrow_flight_sql_static) + list(APPEND ARROW_FLIGHT_SQL_ODBC_SHARED_PRIVATE_LINK_LIBS ODBC::ODBC ${ODBCINST}) +else() + # Unix + set(ARROW_FLIGHT_SQL_ODBC_SHARED_LINK_LIBS arrow_odbc_spi_impl) + set(ARROW_FLIGHT_SQL_ODBC_STATIC_LINK_LIBS ODBC::ODBC ${ODBCINST}) +endif() + add_arrow_lib(arrow_flight_sql_odbc CMAKE_PACKAGE_NAME ArrowFlightSqlOdbc @@ -70,24 +87,22 @@ add_arrow_lib(arrow_flight_sql_odbc ARROW_FLIGHT_SQL_ODBC_LIBRARIES SOURCES ${ARROW_FLIGHT_SQL_ODBC_SRCS} - DEPENDENCIES - arrow_flight_sql DEFINITIONS UNICODE + DEPENDENCIES + ${ARROW_FLIGHT_SQL_ODBC_DEPENDENCIES} SHARED_LINK_FLAGS ${ARROW_VERSION_SCRIPT_FLAGS} # Defined in cpp/arrow/CMakeLists.txt SHARED_LINK_LIBS - arrow_flight_sql_shared + ${ARROW_FLIGHT_SQL_ODBC_SHARED_LINK_LIBS} SHARED_INSTALL_INTERFACE_LIBS - ArrowFlight::arrow_flight_sql_shared + ${ARROW_FLIGHT_SQL_ODBC_SHARED_INSTALL_INTERFACE_LIBS} STATIC_LINK_LIBS - arrow_flight_sql_static + ${ARROW_FLIGHT_SQL_ODBC_STATIC_LINK_LIBS} STATIC_INSTALL_INTERFACE_LIBS - ArrowFlight::arrow_flight_sql_static + ${ARROW_FLIGHT_SQL_ODBC_STATIC_INSTALL_INTERFACE_LIBS} SHARED_PRIVATE_LINK_LIBS - ODBC::ODBC - ${ODBCINST} - arrow_odbc_spi_impl) + ${ARROW_FLIGHT_SQL_ODBC_SHARED_PRIVATE_LINK_LIBS}) 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 e58558258df0..0897248f6cea 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 dd937410adc1..2e7196e322fa 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 634fd77862e9..6518e48459e7 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 179303b68e38..47149a33a560 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 a65800114855..9745f58b2e22 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 bfac68df8b98..74a750537362 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 ef0c7271ec23..7f9ab668a792 100644 --- a/cpp/src/arrow/flight/sql/odbc/tests/CMakeLists.txt +++ b/cpp/src/arrow/flight/sql/odbc/tests/CMakeLists.txt @@ -25,29 +25,57 @@ set(ARROW_FLIGHT_SQL_MOCK_SERVER_SRCS ../../example/sqlite_server.cc ../../example/sqlite_tables_schema_batch_reader.cc) +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) + +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_LIBS ${ODBC_LIBRARIES} ${ODBCINST} ${SQLite3_LIBRARIES}) + +# On Windows, dynamic linking ODBC is supported, tests link libraries dynamically. +# On unix systems, static linking ODBC is supported, thus tests link libraries statically. +set(ARROW_FLIGHT_SQL_ODBC_TEST_EXTRA_LINK_LIBS "") +set(ARROW_FLIGHT_SQL_ODBC_TEST_STATIC_LINK_LIBS "") +if(WIN32) + # arrow_odbc_spi_impl is required on Windows due to dynamic linking + list(APPEND ARROW_FLIGHT_SQL_ODBC_TEST_EXTRA_LINK_LIBS arrow_odbc_spi_impl + ${ARROW_FLIGHT_SQL_ODBC_TEST_LIBS}) +else() + # Unix + list(APPEND ARROW_FLIGHT_SQL_ODBC_TEST_STATIC_LINK_LIBS + ${ARROW_FLIGHT_SQL_ODBC_TEST_LIBS} ${ARROW_FLIGHT_SQL_ODBC_TEST_LINK_LIBS}) +endif() + 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_ODBC_TEST_SRCS} ${ARROW_FLIGHT_SQL_MOCK_SERVER_SRCS} + DEFINITIONS + UNICODE EXTRA_LINK_LIBS - ${ODBC_LIBRARIES} - ${ODBCINST} - ${SQLite3_LIBRARIES} - arrow_odbc_spi_impl) + ${ARROW_FLIGHT_SQL_ODBC_TEST_EXTRA_LINK_LIBS} + STATIC_LINK_LIBS + ${ARROW_FLIGHT_SQL_ODBC_TEST_STATIC_LINK_LIBS}) 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 34a32738455a..36f91f827e4d 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 3115cd627547..419f698c5d96 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 0a4e99d33a6f..c1b021bca659 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)); }