diff --git a/.github/workflows/cpp_extra.yml b/.github/workflows/cpp_extra.yml index 8ebf3b9dfbfe..21b780e4e359 100644 --- a/.github/workflows/cpp_extra.yml +++ b/.github/workflows/cpp_extra.yml @@ -336,6 +336,64 @@ jobs: cd cpp/examples/minimal_build ../minimal_build.build/arrow-example + odbc-linux: + needs: check-labels + name: ODBC Linux + runs-on: ubuntu-latest + if: >- + needs.check-labels.outputs.force == 'true' || + contains(fromJSON(needs.check-labels.outputs.ci-extra-labels || '[]'), 'CI: Extra') || + contains(fromJSON(needs.check-labels.outputs.ci-extra-labels || '[]'), 'CI: Extra: C++') + timeout-minutes: 75 + strategy: + fail-fast: false + env: + ARCH: amd64 + ARCHERY_DEBUG: 1 + ARROW_ENABLE_TIMING_TESTS: OFF + CLANG_TOOLS: 18 + DOCKER_VOLUME_PREFIX: ".docker/" + LLVM: 18 + UBUNTU: 24.04 + steps: + - name: Checkout Arrow + uses: actions/checkout@v6 + with: + fetch-depth: 0 + submodules: recursive + - name: Cache Docker Volumes + uses: actions/cache@v5 + with: + path: .docker + key: ubuntu-cpp-odbc-${{ hashFiles('cpp/**') }} + restore-keys: ubuntu-cpp-odbc- + - name: Setup Python on hosted runner + uses: actions/setup-python@v6 + with: + python-version: 3 + - name: Setup Archery + run: python3 -m pip install -e dev/archery[docker] + - name: Execute Docker Build + env: + ARCHERY_DOCKER_USER: ${{ secrets.DOCKERHUB_USER }} + ARCHERY_DOCKER_PASSWORD: ${{ secrets.DOCKERHUB_TOKEN }} + run: | + # GH-40558: reduce ASLR to avoid ASAN/LSAN crashes + sudo sysctl -w vm.mmap_rnd_bits=28 + source ci/scripts/util_enable_core_dumps.sh + archery docker run ubuntu-cpp-odbc + - name: Docker Push + if: >- + success() && + github.event_name == 'push' && + github.repository == 'apache/arrow' && + github.ref_name == 'main' + env: + ARCHERY_DOCKER_USER: ${{ secrets.DOCKERHUB_USER }} + ARCHERY_DOCKER_PASSWORD: ${{ secrets.DOCKERHUB_TOKEN }} + continue-on-error: true + run: archery docker push ubuntu-cpp-odbc + odbc-macos: needs: check-labels name: ODBC ${{ matrix.build-type }} ${{ matrix.architecture }} macOS ${{ matrix.macos-version }} @@ -435,7 +493,7 @@ jobs: "$(pwd)/build/cpp/${{ matrix.build-type }}/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/${{ matrix.build-type }}/libarrow_flight_sql_odbc.dylib + sudo cpp/src/arrow/flight/sql/odbc/install/unix/install_odbc.sh $(pwd)/build/cpp/${{ matrix.build-type }}/libarrow_flight_sql_odbc.dylib - name: Test shell: bash run: | @@ -698,6 +756,7 @@ jobs: - jni-linux - jni-macos - msvc-arm64 + - odbc-linux - odbc-macos - odbc-msvc - odbc-nightly diff --git a/ci/docker/ubuntu-24.04-cpp.dockerfile b/ci/docker/ubuntu-24.04-cpp.dockerfile index 7703046c75cd..5953c49ff9a0 100644 --- a/ci/docker/ubuntu-24.04-cpp.dockerfile +++ b/ci/docker/ubuntu-24.04-cpp.dockerfile @@ -121,6 +121,7 @@ RUN apt-get update -y -q && \ rsync \ tzdata \ tzdata-legacy \ + unixodbc-dev \ uuid-runtime \ unzip \ wget && \ diff --git a/compose.yaml b/compose.yaml index c799059fe254..407253b7b77c 100644 --- a/compose.yaml +++ b/compose.yaml @@ -151,6 +151,7 @@ x-hierarchy: - ubuntu-r-only-r - ubuntu-cpp-bundled - ubuntu-cpp-bundled-offline + - ubuntu-cpp-odbc - ubuntu-cpp-minimal - ubuntu-cuda-cpp: - ubuntu-cuda-python @@ -496,6 +497,43 @@ services: volumes: *ubuntu-volumes command: *cpp-command + ubuntu-cpp-odbc: + # Arrow Flight SQL ODBC build with BUNDLED dependencies with downloaded dependencies. + image: ${REPO}:${ARCH}-ubuntu-${UBUNTU}-cpp + build: + context: . + dockerfile: ci/docker/ubuntu-${UBUNTU}-cpp.dockerfile + cache_from: + - ${REPO}:${ARCH}-ubuntu-${UBUNTU}-cpp + args: + arch: ${ARCH} + base: "${ARCH}/ubuntu:${UBUNTU}" + clang_tools: ${CLANG_TOOLS} + cmake: ${CMAKE} + gcc: ${GCC} + llvm: ${LLVM} + shm_size: *shm-size + cap_add: + - SYS_ADMIN + devices: + - "/dev/fuse:/dev/fuse" + security_opt: + - apparmor:unconfined + ulimits: *ulimits + environment: + <<: [*common, *ccache, *sccache, *cpp] + ARROW_BUILD_TYPE: RELEASE + ARROW_DEPENDENCY_SOURCE: BUNDLED + ARROW_DEPENDENCY_USE_SHARED: OFF + ARROW_FLIGHT_SQL_ODBC: ON + volumes: *ubuntu-volumes + # Register ODBC before running tests + command: > + /bin/bash -c " + /arrow/ci/scripts/cpp_build.sh /arrow /build && + sudo /arrow/cpp/src/arrow/flight/sql/odbc/install/unix/install_odbc.sh /usr/local/lib/libarrow_flight_sql_odbc.so && + /arrow/ci/scripts/cpp_test.sh /arrow /build" + ubuntu-cpp-minimal: # Arrow build with minimal components/dependencies image: ${REPO}:${ARCH}-ubuntu-${UBUNTU}-cpp-minimal diff --git a/cpp/cmake_modules/DefineOptions.cmake b/cpp/cmake_modules/DefineOptions.cmake index 5d34ff50e35c..017a5a6efb26 100644 --- a/cpp/cmake_modules/DefineOptions.cmake +++ b/cpp/cmake_modules/DefineOptions.cmake @@ -107,10 +107,6 @@ macro(tsort_bool_option_dependencies) endmacro() macro(resolve_option_dependencies) - # Arrow Flight SQL ODBC is available only for Windows and macOS for now. - if(NOT WIN32 AND NOT APPLE) - set(ARROW_FLIGHT_SQL_ODBC OFF) - endif() if(MSVC_TOOLCHAIN) set(ARROW_USE_GLOG OFF) endif() diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index 8742bcb55d90..3041ee9ba2bb 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -1090,7 +1090,7 @@ function(build_boost) set(ARROW_BOOST_NEED_MULTIPRECISION FALSE) endif() if(ARROW_ENABLE_THREADING) - if(ARROW_WITH_THRIFT OR (ARROW_FLIGHT_SQL_ODBC AND MSVC)) + if(ARROW_WITH_THRIFT OR ARROW_FLIGHT_SQL_ODBC) list(APPEND BOOST_INCLUDE_LIBRARIES locale) endif() if(ARROW_BOOST_NEED_MULTIPRECISION) diff --git a/cpp/src/arrow/flight/sql/odbc/CMakeLists.txt b/cpp/src/arrow/flight/sql/odbc/CMakeLists.txt index 2560cdccd09e..4227873706ff 100644 --- a/cpp/src/arrow/flight/sql/odbc/CMakeLists.txt +++ b/cpp/src/arrow/flight/sql/odbc/CMakeLists.txt @@ -28,7 +28,12 @@ else() endif() add_subdirectory(odbc_impl) -add_subdirectory(tests) +if(ARROW_BUILD_TESTS) + if(WIN32 OR APPLE) + # GH-49552 TODO: Enable Linux test build + add_subdirectory(tests) + endif() +endif() arrow_install_all_headers("arrow/flight/sql/odbc") diff --git a/cpp/src/arrow/flight/sql/odbc/install/mac/install_odbc.sh b/cpp/src/arrow/flight/sql/odbc/install/unix/install_odbc.sh similarity index 93% rename from cpp/src/arrow/flight/sql/odbc/install/mac/install_odbc.sh rename to cpp/src/arrow/flight/sql/odbc/install/unix/install_odbc.sh index 069c534c2973..a86f71caae51 100755 --- a/cpp/src/arrow/flight/sql/odbc/install/mac/install_odbc.sh +++ b/cpp/src/arrow/flight/sql/odbc/install/unix/install_odbc.sh @@ -40,7 +40,16 @@ if [ ! -f "$ODBC_64BIT" ]; then exit 1 fi -USER_ODBCINST_FILE="$HOME/Library/ODBC/odbcinst.ini" +case "$(uname)" in + Linux) + USER_ODBCINST_FILE="/etc/odbcinst.ini" + ;; + *) + # macOS + USER_ODBCINST_FILE="$HOME/Library/ODBC/odbcinst.ini" + ;; +esac + DRIVER_NAME="Apache Arrow Flight SQL ODBC Driver" mkdir -p "$HOME"/Library/ODBC diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_api.cc b/cpp/src/arrow/flight/sql/odbc/odbc_api.cc index 75b35b688aa6..7c64dee21f5b 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_api.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_api.cc @@ -37,6 +37,8 @@ #endif // defined(_WIN32) namespace arrow::flight::sql::odbc { +void LoadPropertiesFromDSN(const std::string& dsn, Connection::ConnPropertyMap& props); + SQLRETURN SQLAllocHandle(SQLSMALLINT type, SQLHANDLE parent, SQLHANDLE* result) { ARROW_LOG(DEBUG) << "SQLAllocHandle called with type: " << type << ", parent: " << parent diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_api_internal.h b/cpp/src/arrow/flight/sql/odbc/odbc_api_internal.h index f9d8d887cb87..0f581d734e1d 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_api_internal.h +++ b/cpp/src/arrow/flight/sql/odbc/odbc_api_internal.h @@ -20,8 +20,7 @@ #include "arrow/flight/sql/odbc/odbc_impl/platform.h" #include -#include -#include +#include // \file odbc_api_internal.h // 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 74c60cd91632..5a16c0361f32 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/CMakeLists.txt +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/CMakeLists.txt @@ -130,7 +130,14 @@ if(WIN32) win_system_dsn.cc) endif() -if(APPLE) +if(WIN32) + find_package(ODBC REQUIRED) + target_include_directories(arrow_odbc_spi_impl PUBLIC ${ODBC_INCLUDE_DIR}) + target_link_libraries(arrow_odbc_spi_impl + PUBLIC arrow_flight_sql_shared arrow_compute_shared Boost::locale + ${ODBCINST}) +else() + # Unix target_include_directories(arrow_odbc_spi_impl SYSTEM BEFORE PUBLIC ${ODBC_INCLUDE_DIR}) target_link_libraries(arrow_odbc_spi_impl PUBLIC arrow_flight_sql_static @@ -138,12 +145,11 @@ if(APPLE) Boost::locale Boost::headers RapidJSON) -else() - find_package(ODBC REQUIRED) - target_include_directories(arrow_odbc_spi_impl PUBLIC ${ODBC_INCLUDE_DIR}) - target_link_libraries(arrow_odbc_spi_impl - PUBLIC arrow_flight_sql_shared arrow_compute_shared Boost::locale - ${ODBCINST}) + + if(NOT APPLE) + # Explicitly link to unix-odbc on Linux + target_link_libraries(arrow_odbc_spi_impl PUBLIC ${ODBCINST}) + endif() endif() # CLI diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/binary_array_accessor_test.cc b/cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/binary_array_accessor_test.cc index 39d03692da6c..55ef46458e21 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/binary_array_accessor_test.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/binary_array_accessor_test.cc @@ -42,7 +42,7 @@ TEST(BinaryArrayAccessor, Test_CDataType_BINARY_Basic) { accessor.GetColumnarData(&binding, 0, values.size(), value_offset, false, diagnostics, nullptr)); - for (int i = 0; i < values.size(); ++i) { + for (size_t i = 0; i < values.size(); ++i) { ASSERT_EQ(values[i].length(), str_len_buffer[i]); // Beware that CDataType_BINARY values are not null terminated. // It's safe to create a std::string from this data because we know it's diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/boolean_array_accessor_test.cc b/cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/boolean_array_accessor_test.cc index 9b17a9045981..68583b3f15e0 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/boolean_array_accessor_test.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/boolean_array_accessor_test.cc @@ -39,7 +39,7 @@ TEST(BooleanArrayFlightSqlAccessor, Test_BooleanArray_CDataType_BIT) { accessor.GetColumnarData(&binding, 0, values.size(), value_offset, false, diagnostics, nullptr)); - for (int i = 0; i < values.size(); ++i) { + for (size_t i = 0; i < values.size(); ++i) { ASSERT_EQ(sizeof(unsigned char), str_len_buffer[i]); ASSERT_EQ(values[i] ? 1 : 0, buffer[i]); } diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/decimal_array_accessor_test.cc b/cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/decimal_array_accessor_test.cc index b2eb9450c2fa..9833ea1d59c7 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/decimal_array_accessor_test.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/decimal_array_accessor_test.cc @@ -83,7 +83,7 @@ void AssertNumericOutput(int input_precision, int input_scale, accessor.GetColumnarData(&binding, 0, values.size(), value_offset, false, diagnostics, nullptr)); - for (int i = 0; i < values.size(); ++i) { + for (size_t i = 0; i < values.size(); ++i) { ASSERT_EQ(sizeof(NUMERIC_STRUCT), str_len_buffer[i]); ASSERT_EQ(output_precision, buffer[i].precision); diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/primitive_array_accessor_test.cc b/cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/primitive_array_accessor_test.cc index 2f04b7324a56..bd9ecf3314fd 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/primitive_array_accessor_test.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/primitive_array_accessor_test.cc @@ -46,7 +46,7 @@ void TestPrimitiveArraySqlAccessor() { accessor.GetColumnarData(&binding, 0, values.size(), value_offset, false, diagnostics, nullptr)); - for (int i = 0; i < values.size(); ++i) { + for (size_t i = 0; i < values.size(); ++i) { ASSERT_EQ(sizeof(c_type), str_len_buffer[i]); ASSERT_EQ(values[i], buffer[i]); } diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/string_array_accessor_test.cc b/cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/string_array_accessor_test.cc index eb7a9c88b3b3..9316571dd7b3 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/string_array_accessor_test.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/string_array_accessor_test.cc @@ -43,7 +43,7 @@ TEST(StringArrayAccessor, Test_CDataType_CHAR_Basic) { accessor.GetColumnarData(&binding, 0, values.size(), value_offset, false, diagnostics, nullptr)); - for (int i = 0; i < values.size(); ++i) { + for (size_t i = 0; i < values.size(); ++i) { ASSERT_EQ(values[i].length(), str_len_buffer[i]); ASSERT_EQ(values[i], std::string(buffer.data() + i * max_str_len)); } @@ -102,7 +102,7 @@ TEST(StringArrayAccessor, Test_CDataType_WCHAR_Basic) { accessor->GetColumnarData(&binding, 0, values.size(), value_offset, false, diagnostics, nullptr)); - for (int i = 0; i < values.size(); ++i) { + for (size_t i = 0; i < values.size(); ++i) { ASSERT_EQ(values[i].length() * GetSqlWCharSize(), str_len_buffer[i]); std::vector expected; Utf8ToWcs(values[i].c_str(), &expected); diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/types.h b/cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/types.h index ca33d872fa77..f75d2894af99 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/types.h +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/types.h @@ -43,12 +43,12 @@ struct ColumnBinding { ColumnBinding(CDataType target_type, int precision, int scale, void* buffer, size_t buffer_length, ssize_t* str_len_buffer) - : target_type(target_type), - precision(precision), - scale(scale), - buffer(buffer), + : buffer(buffer), + str_len_buffer(str_len_buffer), buffer_length(buffer_length), - str_len_buffer(str_len_buffer) {} + target_type(target_type), + precision(precision), + scale(scale) {} }; /// \brief Accessor interface meant to provide a way of populating data of a diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/attribute_utils.h b/cpp/src/arrow/flight/sql/odbc/odbc_impl/attribute_utils.h index 315d854b60d5..cc1521523dcc 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/attribute_utils.h +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/attribute_utils.h @@ -158,11 +158,21 @@ inline void SetAttributeSQLWCHAR(SQLPOINTER new_value, SQLINTEGER input_length_i thread_local std::vector utf8_str; if (input_length_in_bytes == SQL_NTS) { arrow::flight::sql::odbc::WcsToUtf8(new_value, &utf8_str); + } else if (input_length_in_bytes <= 0) { + // empty string + attribute_to_write.clear(); + return; } else { arrow::flight::sql::odbc::WcsToUtf8( new_value, input_length_in_bytes / arrow::flight::sql::odbc::GetSqlWCharSize(), &utf8_str); } + + // add null-terminator + if (utf8_str.back() != '\0') { + utf8_str.push_back('\0'); + } + attribute_to_write.assign((char*)utf8_str.data()); } 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 e52c305e4616..55ec3a7beaec 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 @@ -49,7 +49,7 @@ class BlockingQueue { void AddProducer(Supplier supplier) { active_threads_++; - threads_.emplace_back([=] { + threads_.emplace_back([this, supplier] { while (!closed_) { // Block while queue is full std::unique_lock unique_lock(mtx_); diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/config/configuration.cc b/cpp/src/arrow/flight/sql/odbc/odbc_impl/config/configuration.cc index 0e49613979ac..e18ab0bae8d2 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/config/configuration.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/config/configuration.cc @@ -15,8 +15,12 @@ // specific language governing permissions and limitations // under the License. -#include "arrow/flight/sql/odbc/odbc_impl/config/configuration.h" +// flight_sql_connection.h needs to be included first due to conflicts with windows.h #include "arrow/flight/sql/odbc/odbc_impl/flight_sql_connection.h" + +#include "arrow/flight/sql/odbc/odbc_impl/config/configuration.h" + +#include "arrow/flight/sql/odbc/odbc_impl/attribute_utils.h" #include "arrow/flight/sql/odbc/odbc_impl/util.h" #include "arrow/result.h" #include "arrow/util/utf8.h" @@ -27,6 +31,8 @@ #include #include +using ODBC::SetAttributeSQLWCHAR; + namespace arrow::flight::sql::odbc { namespace config { static const char DEFAULT_DSN[] = "Apache Arrow Flight SQL"; @@ -35,28 +41,30 @@ static const char DEFAULT_USE_CERT_STORE[] = TRUE_STR; static const char DEFAULT_DISABLE_CERT_VERIFICATION[] = FALSE_STR; namespace { -std::string ReadDsnString(const std::string& dsn, std::string_view key, +std::string ReadDsnString(const std::string& dsn, const std::string_view& key, const std::string& dflt = "") { - CONVERT_WIDE_STR(const std::wstring wdsn, dsn); - CONVERT_WIDE_STR(const std::wstring wkey, key); - CONVERT_WIDE_STR(const std::wstring wdflt, dflt); + CONVERT_SQLWCHAR_STR(wdsn, dsn); + CONVERT_SQLWCHAR_STR(wkey, key); + CONVERT_SQLWCHAR_STR(wdflt, dflt); #define BUFFER_SIZE (1024) - std::vector buf(BUFFER_SIZE); - int ret = - SQLGetPrivateProfileString(wdsn.c_str(), wkey.c_str(), wdflt.c_str(), buf.data(), - static_cast(buf.size()), L"ODBC.INI"); + std::vector buf(BUFFER_SIZE); + int ret = SQLGetPrivateProfileString( + reinterpret_cast(wdsn.c_str()), reinterpret_cast(wkey.c_str()), + reinterpret_cast(wdflt.c_str()), buf.data(), static_cast(buf.size()), + ODBC_INI); if (ret > BUFFER_SIZE) { // If there wasn't enough space, try again with the right size buffer. buf.resize(ret + 1); - ret = - SQLGetPrivateProfileString(wdsn.c_str(), wkey.c_str(), wdflt.c_str(), buf.data(), - static_cast(buf.size()), L"ODBC.INI"); + ret = SQLGetPrivateProfileString(reinterpret_cast(wdsn.c_str()), + reinterpret_cast(wkey.c_str()), + reinterpret_cast(wdflt.c_str()), buf.data(), + static_cast(buf.size()), ODBC_INI); } - std::wstring wresult = std::wstring(buf.data(), ret); - CONVERT_UTF8_STR(const std::string result, wresult); + std::string result(""); + SetAttributeSQLWCHAR(buf.data(), ret * GetSqlWCharSize(), result); return result; } @@ -74,31 +82,35 @@ void RemoveAllKnownKeys(std::vector& keys) { } std::vector ReadAllKeys(const std::string& dsn) { - CONVERT_WIDE_STR(const std::wstring wdsn, dsn); + CONVERT_SQLWCHAR_STR(wdsn, dsn); - std::vector buf(BUFFER_SIZE); + std::vector buf(BUFFER_SIZE); - int ret = SQLGetPrivateProfileString(wdsn.c_str(), NULL, L"", buf.data(), - static_cast(buf.size()), L"ODBC.INI"); + int ret = SQLGetPrivateProfileString(reinterpret_cast(wdsn.c_str()), NULL, + reinterpret_cast(L""), buf.data(), + static_cast(buf.size()), ODBC_INI); if (ret > BUFFER_SIZE) { // If there wasn't enough space, try again with the right size buffer. buf.resize(ret + 1); - ret = SQLGetPrivateProfileString(wdsn.c_str(), NULL, L"", buf.data(), - static_cast(buf.size()), L"ODBC.INI"); + ret = SQLGetPrivateProfileString(reinterpret_cast(wdsn.c_str()), NULL, + reinterpret_cast(L""), buf.data(), + static_cast(buf.size()), ODBC_INI); } // When you pass NULL to SQLGetPrivateProfileString it gives back a \0 delimited list of // all the keys. The below loop simply tokenizes all the keys and places them into a // vector. std::vector keys; - wchar_t* begin = buf.data(); + SQLWCHAR* begin = buf.data(); while (begin && *begin != '\0') { - wchar_t* cur; + SQLWCHAR* cur; for (cur = begin; *cur != '\0'; ++cur) { } - CONVERT_UTF8_STR(const std::string key, std::wstring(begin, cur)); + std::string key(""); + SQLINTEGER key_len = static_cast(cur - begin); + SetAttributeSQLWCHAR(begin, key_len * GetSqlWCharSize(), key); keys.emplace_back(key); begin = ++cur; } @@ -123,6 +135,10 @@ void Configuration::LoadDefaults() { } void Configuration::LoadDsn(const std::string& dsn) { + // Read keys before reading DSN to minimized unexpected behavior from ODBC driver + // managers. + auto customKeys = ReadAllKeys(dsn); + Set(FlightSqlConnection::DSN, dsn); Set(FlightSqlConnection::HOST, ReadDsnString(dsn, FlightSqlConnection::HOST)); Set(FlightSqlConnection::PORT, ReadDsnString(dsn, FlightSqlConnection::PORT)); @@ -131,6 +147,7 @@ void Configuration::LoadDsn(const std::string& dsn) { Set(FlightSqlConnection::PWD, ReadDsnString(dsn, FlightSqlConnection::PWD)); Set(FlightSqlConnection::TRUSTED_CERTS, ReadDsnString(dsn, FlightSqlConnection::TRUSTED_CERTS)); + #ifdef __APPLE__ // macOS iODBC treats non-empty defaults as the real values when reading from system // DSN, so we don't pass defaults on macOS. @@ -151,9 +168,8 @@ void Configuration::LoadDsn(const std::string& dsn) { Set(FlightSqlConnection::DISABLE_CERTIFICATE_VERIFICATION, ReadDsnString(dsn, FlightSqlConnection::DISABLE_CERTIFICATE_VERIFICATION, DEFAULT_DISABLE_CERT_VERIFICATION)); -#endif +#endif // __APPLE__ - auto customKeys = ReadAllKeys(dsn); RemoveAllKnownKeys(customKeys); for (auto key : customKeys) { std::string_view key_sv(key); diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/encoding_utils.h b/cpp/src/arrow/flight/sql/odbc/odbc_impl/encoding_utils.h index 7f8a4a7ef856..61a728d0a722 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/encoding_utils.h +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/encoding_utils.h @@ -28,8 +28,26 @@ #include #include +// Workaround for ODBC `BOOL` def conflict on Linux +#ifdef __linux__ +# ifdef BOOL +# undef BOOL +# endif // BOOL +#endif // __linux__ +// Include fwd.h headers after ODBC headers +#include "arrow/flight/sql/odbc/odbc_impl/util.h" + #define _SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING +#ifdef __linux__ +# define GET_SQWCHAR_PTR(wstring_var) (ODBC::ToSqlWCharVector(wstring_var).data()) +#else +// Windows and macOS +# define GET_SQWCHAR_PTR(wstring_var) (wstring_var.c_str()) +#endif + +#define ODBC_INI reinterpret_cast(GET_SQWCHAR_PTR(std::wstring(L"ODBC.INI"))) + namespace ODBC { // Return the number of bytes required for the conversion. @@ -117,4 +135,27 @@ inline std::string SqlStringToString(const unsigned char* sql_str, return res; } + +inline std::vector ToSqlWCharVector(const std::wstring& ws) { + switch (arrow::flight::sql::odbc::GetSqlWCharSize()) { + case sizeof(wchar_t): { + return std::vector(ws.begin(), ws.end()); + } +#ifdef __linux__ + case sizeof(char16_t): { + // Linux ODBC driver manager uses char16_t as SQLWCHAR + CONVERT_UTF8_STR(const std::string utf8s, ws); + CONVERT_UTF16_STR(const std::u16string utf16s, utf8s); + return std::vector(utf16s.begin(), utf16s.end()); + } +#endif // __linux__ + default: { + assert(false); + throw arrow::flight::sql::odbc::DriverException( + "Encoding is unsupported, SQLWCHAR size: " + + std::to_string(arrow::flight::sql::odbc::GetSqlWCharSize())); + } + } +} + } // namespace ODBC 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 8b2b564d8db8..f00ce85d9f3e 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 @@ -27,6 +27,7 @@ #include "arrow/flight/sql/odbc/odbc_impl/util.h" #include "arrow/flight/types.h" +#define BOOST_NO_CXX98_FUNCTION_BASE // ARROW-17805 #include #include #include @@ -297,7 +298,7 @@ FlightClientOptions FlightSqlConnection::BuildFlightClientOptions( } } - return std::move(options); + return options; } Location FlightSqlConnection::BuildLocation( @@ -409,9 +410,9 @@ Connection::Info FlightSqlConnection::GetInfo(uint16_t info_type) { FlightSqlConnection::FlightSqlConnection(OdbcVersion odbc_version, const std::string& driver_version) - : diagnostics_("Apache Arrow", "Flight SQL", odbc_version), + : info_(client_options_, call_options_, sql_client_, driver_version), + diagnostics_("Apache Arrow", "Flight SQL", odbc_version), odbc_version_(odbc_version), - info_(client_options_, call_options_, sql_client_, driver_version), closed_(true) { attribute_[CONNECTION_DEAD] = static_cast(SQL_TRUE); attribute_[LOGIN_TIMEOUT] = static_cast(0); diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_result_set.cc b/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_result_set.cc index 56e5bb973f75..fb743d1c1ec0 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_result_set.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_result_set.cc @@ -81,7 +81,7 @@ size_t FlightSqlResultSet::Move(size_t rows, size_t bind_offset, size_t bind_typ } // Reset GetData value offsets. - if (num_binding_ != get_data_offsets_.size() && reset_get_data_) { + if (static_cast(num_binding_) != get_data_offsets_.size() && reset_get_data_) { std::fill(get_data_offsets_.begin(), get_data_offsets_.end(), 0); } diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_result_set_accessors.cc b/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_result_set_accessors.cc index f309ab6156ea..e34b36ab0d43 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_result_set_accessors.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_result_set_accessors.cc @@ -29,6 +29,13 @@ #include "arrow/flight/sql/odbc/odbc_impl/accessors/timestamp_array_accessor.h" #include "arrow/flight/sql/odbc/odbc_impl/platform.h" +// Workaround for ODBC `BOOL` def conflict on Linux +#ifdef __linux__ +# ifdef BOOL +# undef BOOL +# endif // BOOL +#endif // __linux__ + namespace arrow::flight::sql::odbc { typedef std::pair SourceAndTargetPair; 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 c35154b44eab..46456e8c3c91 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 @@ -58,9 +58,9 @@ FlightSqlStatement::FlightSqlStatement(const Diagnostics& diagnostics, const MetadataSettings& metadata_settings) : diagnostics_("Apache Arrow", diagnostics.GetDataSourceComponent(), diagnostics.GetOdbcVersion()), - sql_client_(sql_client), client_options_(std::move(client_options)), call_options_(std::move(call_options)), + sql_client_(sql_client), metadata_settings_(metadata_settings) { attribute_[METADATA_ID] = static_cast(SQL_FALSE); attribute_[MAX_LENGTH] = static_cast(0); @@ -88,8 +88,8 @@ bool FlightSqlStatement::SetAttribute(StatementAttributeId attribute, TimeoutDuration{static_cast(boost::get(value))}; } else { call_options_.timeout = TimeoutDuration{-1}; - // Intentional fall-through. } + [[fallthrough]]; default: attribute_[attribute] = value; return true; 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 7d6239f24a7e..6923d7fafbe4 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 @@ -395,8 +395,9 @@ bool GetInfoCache::LoadInfoFromServer() { // Unused by ODBC. break; case SqlInfoOptions::SQL_DDL_SCHEMA: { - bool supports_schema_ddl = - reinterpret_cast(scalar->child_value().get())->value; + // GH-49500 TODO: use scalar bool to determine `SQL_CREATE_SCHEMA` and + // `SQL_DROP_SCHEMA` values + // Note: this is a bitmask and we can't describe cascade or restrict // flags. info_[SQL_DROP_SCHEMA] = static_cast(SQL_DS_DROP_SCHEMA); @@ -407,8 +408,9 @@ bool GetInfoCache::LoadInfoFromServer() { break; } case SqlInfoOptions::SQL_DDL_TABLE: { - bool supports_table_ddl = - reinterpret_cast(scalar->child_value().get())->value; + // GH-49500 TODO: use scalar bool to determine `SQL_CREATE_TABLE` and + // `SQL_DROP_TABLE` values + // This is a bitmask and we cannot describe all clauses. info_[SQL_CREATE_TABLE] = static_cast(SQL_CT_CREATE_TABLE); info_[SQL_DROP_TABLE] = static_cast(SQL_DT_DROP_TABLE); @@ -697,7 +699,6 @@ bool GetInfoCache::LoadInfoFromServer() { break; } case SqlInfoOptions::SQL_DEFAULT_TRANSACTION_ISOLATION: { - constexpr int32_t NONE = 0; constexpr int32_t READ_UNCOMMITTED = 1; constexpr int32_t READ_COMMITTED = 2; constexpr int32_t REPEATABLE_READ = 3; diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/main.cc b/cpp/src/arrow/flight/sql/odbc/odbc_impl/main.cc index 3336e0160e1f..47929830067a 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/main.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/main.cc @@ -69,7 +69,7 @@ void TestBindColumn(const std::shared_ptr& connection) { total += fetched_rows; std::cout << "Total:" << total << std::endl; - for (int i = 0; i < fetched_rows; ++i) { + for (size_t i = 0; i < fetched_rows; ++i) { ARROW_LOG(DEBUG) << "Row[" << i << "] incidnt_num: '" << incidnt_num[i] << "', Category: '" << category[i] << "'"; } @@ -138,7 +138,7 @@ void TestBindColumnBigInt(const std::shared_ptr& connection) { total += fetched_rows; ARROW_LOG(DEBUG) << "Total:" << total; - for (int i = 0; i < fetched_rows; ++i) { + for (size_t i = 0; i < fetched_rows; ++i) { ARROW_LOG(DEBUG) << "Row[" << i << "] incidnt_num: '" << incidnt_num[i] << "', " << "double_field: '" << double_field[i] << "', " << "category: '" << category[i] << "'"; @@ -183,7 +183,7 @@ void TestGetColumnsV3(const std::shared_ptr& connection) { ssize_t result_length; while (result_set->Move(1, 0, 0, nullptr) == 1) { - for (int i = 0; i < column_count; ++i) { + for (size_t i = 0; i < column_count; ++i) { result_set->GetData(1 + i, arrow::flight::sql::odbc::CDataType_CHAR, 0, 0, result.data(), buffer_length, &result_length); std::cout << (result_length != -1 ? result.data() : "NULL") << '\t'; 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 59cdc2e12d90..142dac53ab6f 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 @@ -15,11 +15,12 @@ // specific language governing permissions and limitations // under the License. -#include "arrow/flight/sql/odbc/odbc_impl/odbc_connection.h" - #include "arrow/result.h" #include "arrow/util/utf8.h" +// Include ODBC headers after arrow fwd type header to avoid conflicts +#include "arrow/flight/sql/odbc/odbc_impl/odbc_connection.h" + #include "arrow/flight/sql/odbc/odbc_impl/attribute_utils.h" #include "arrow/flight/sql/odbc/odbc_impl/exceptions.h" #include "arrow/flight/sql/odbc/odbc_impl/odbc_descriptor.h" @@ -29,10 +30,12 @@ #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 ODBC headers after arrow headers to avoid conflicts #include #include #include + +#define BOOST_NO_CXX98_FUNCTION_BASE // ARROW-17805 #include #include #include diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/odbc_descriptor.cc b/cpp/src/arrow/flight/sql/odbc/odbc_impl/odbc_descriptor.cc index eae90ac2b77a..e54fbf601eb6 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/odbc_descriptor.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/odbc_descriptor.cc @@ -17,8 +17,6 @@ #include "arrow/flight/sql/odbc/odbc_impl/odbc_descriptor.h" -#include -#include #include #include "arrow/flight/sql/odbc/odbc_impl/attribute_utils.h" #include "arrow/flight/sql/odbc/odbc_impl/exceptions.h" @@ -28,6 +26,10 @@ #include "arrow/flight/sql/odbc/odbc_impl/spi/statement.h" #include "arrow/flight/sql/odbc/odbc_impl/type_utilities.h" +// Include ODBC headers after arrow headers to avoid conflicts +#include +#include + using ODBC::DescriptorRecord; using ODBC::ODBCConnection; using ODBC::ODBCDescriptor; @@ -155,7 +157,7 @@ void ODBCDescriptor::SetField(SQLSMALLINT record_number, SQLSMALLINT field_ident throw DriverException("Bookmarks are unsupported.", "07009"); } - if (record_number > records_.size()) { + if (static_cast(record_number) > records_.size()) { throw DriverException("Invalid descriptor index", "HY009"); } @@ -308,7 +310,7 @@ void ODBCDescriptor::GetField(SQLSMALLINT record_number, SQLSMALLINT field_ident throw DriverException("Bookmarks are unsupported.", "07009"); } - if (record_number > records_.size()) { + if (static_cast(record_number) > records_.size()) { throw DriverException("Invalid descriptor index", "07009"); } @@ -539,7 +541,7 @@ void ODBCDescriptor::BindCol(SQLSMALLINT record_number, SQLSMALLINT c_type, assert(is_writable_); // The set of records auto-expands to the supplied record number. - if (records_.size() < record_number) { + if (records_.size() < static_cast(record_number)) { records_.resize(record_number); } @@ -559,7 +561,7 @@ void ODBCDescriptor::BindCol(SQLSMALLINT record_number, SQLSMALLINT c_type, } void ODBCDescriptor::SetDataPtrOnRecord(SQLPOINTER data_ptr, SQLSMALLINT record_number) { - assert(record_number <= records_.size()); + assert(static_cast(record_number) <= records_.size()); DescriptorRecord& record = records_[record_number - 1]; if (data_ptr) { record.CheckConsistency(); 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 a19797e80f34..51152c64782e 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 @@ -15,17 +15,21 @@ // specific language governing permissions and limitations // under the License. -#include "arrow/flight/sql/odbc/odbc_impl/odbc_statement.h" +// platform.h platform.h includes windows.h so it needs to be included first +#include "arrow/flight/sql/odbc/odbc_impl/platform.h" + +#include "arrow/type.h" +// Include ODBC headers after arrow fwd type header to avoid conflicts #include "arrow/flight/sql/odbc/odbc_impl/attribute_utils.h" #include "arrow/flight/sql/odbc/odbc_impl/exceptions.h" #include "arrow/flight/sql/odbc/odbc_impl/odbc_connection.h" #include "arrow/flight/sql/odbc/odbc_impl/odbc_descriptor.h" +#include "arrow/flight/sql/odbc/odbc_impl/odbc_statement.h" #include "arrow/flight/sql/odbc/odbc_impl/spi/result_set.h" #include "arrow/flight/sql/odbc/odbc_impl/spi/result_set_metadata.h" #include "arrow/flight/sql/odbc/odbc_impl/spi/statement.h" #include "arrow/flight/sql/odbc/odbc_impl/types.h" -#include "arrow/type.h" #include #include @@ -737,7 +741,7 @@ SQLRETURN ODBCStatement::GetData(SQLSMALLINT record_number, SQLSMALLINT c_type, SQLLEN* indicator_ptr) { if (record_number == 0) { throw DriverException("Bookmarks are not supported", "07009"); - } else if (record_number > ird_->GetRecords().size()) { + } else if (static_cast(record_number) > ird_->GetRecords().size()) { throw DriverException("Invalid column index: " + std::to_string(record_number), "07009"); } @@ -752,7 +756,7 @@ SQLRETURN ODBCStatement::GetData(SQLSMALLINT record_number, SQLSMALLINT c_type, int scale = ird_record.scale; if (c_type == SQL_ARD_TYPE) { - if (record_number > current_ard_->GetRecords().size()) { + if (static_cast(record_number) > current_ard_->GetRecords().size()) { throw DriverException("Invalid column index: " + std::to_string(record_number), "07009"); } @@ -765,7 +769,7 @@ SQLRETURN ODBCStatement::GetData(SQLSMALLINT record_number, SQLSMALLINT c_type, // Note: this is intentionally not an else if, since the type can be SQL_C_DEFAULT in // the ARD. if (evaluated_c_type == SQL_C_DEFAULT) { - if (record_number <= current_ard_->GetRecords().size()) { + if (static_cast(record_number) <= current_ard_->GetRecords().size()) { const DescriptorRecord& ard_record = current_ard_->GetRecords()[record_number - 1]; precision = ard_record.precision; scale = ard_record.scale; diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/odbc_statement.h b/cpp/src/arrow/flight/sql/odbc/odbc_impl/odbc_statement.h index 7f974b0a85e6..b5ee264f5544 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/odbc_statement.h +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/odbc_statement.h @@ -84,7 +84,7 @@ class ODBCStatement : public ODBCHandle { /// \brief Return number of columns from data set void GetColumnCount(SQLSMALLINT* column_count_ptr); - /// \brief Return number of rows affected by an UPDATE, INSERT, or DELETE statement\ + /// \brief Return number of rows affected by an UPDATE, INSERT, or DELETE statement /// /// -1 is returned as driver only supports SELECT statement void GetRowCount(SQLLEN* row_count_ptr); 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 541848916934..42dc4d8edb15 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 @@ -17,6 +17,7 @@ #pragma once +#define BOOST_NO_CXX98_FUNCTION_BASE // ARROW-17805 #include #include #include @@ -36,7 +37,11 @@ struct CaseInsensitiveComparator { using is_transparent = std::true_type; bool operator()(std::string_view s1, std::string_view s2) const { - return boost::lexicographical_compare(s1, s2, boost::is_iless()); + return std::lexicographical_compare( + s1.begin(), s1.end(), s2.begin(), s2.end(), [](char a, char b) { + return std::tolower(static_cast(a)) < + std::tolower(static_cast(b)); + }); } }; 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 240c51b67202..bdfeea3b79da 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 @@ -22,6 +22,7 @@ #include #include +#include "arrow/flight/sql/odbc/odbc_impl/diagnostics.h" #include "arrow/flight/sql/odbc/odbc_impl/type_fwd.h" namespace arrow::flight::sql::odbc { diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/system_dsn.cc b/cpp/src/arrow/flight/sql/odbc/odbc_impl/system_dsn.cc index 7b974cc35a3e..97c3a52d6c9f 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/system_dsn.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/system_dsn.cc @@ -23,6 +23,29 @@ #include #include +#include "arrow/flight/sql/odbc/odbc_impl/encoding_utils.h" + +#ifdef __linux__ +// Linux driver manager uses utf16string +# define CONVERT_UTF8_TO_SQLWCHAR_OR_RETURN(wvar, var) \ + auto wvar##_result = arrow::util::UTF8StringToUTF16(var); \ + if (!wvar##_result.status().ok()) { \ + PostArrowUtilError(wvar##_result.status()); \ + return false; \ + } \ + std::u16string wvar = wvar##_result.ValueOrDie(); + +#else +// Windows and macOS +# define CONVERT_UTF8_TO_SQLWCHAR_OR_RETURN(wvar, var) \ + auto wvar##_result = arrow::util::UTF8ToWideString(var); \ + if (!wvar##_result.status().ok()) { \ + PostArrowUtilError(wvar##_result.status()); \ + return false; \ + } \ + std::wstring wvar = wvar##_result.ValueOrDie(); +#endif // __linux__ + namespace arrow::flight::sql::odbc { using config::Configuration; @@ -39,20 +62,29 @@ void PostArrowUtilError(arrow::Status error_status) { std::wstring werror_msg = arrow::util::UTF8ToWideString(error_msg).ValueOr( L"Error during utf8 to wide string conversion"); - PostError(ODBC_ERROR_GENERAL_ERR, const_cast(werror_msg.c_str())); + PostError(ODBC_ERROR_GENERAL_ERR, const_cast(GET_SQWCHAR_PTR(werror_msg))); } void PostLastInstallerError() { #define BUFFER_SIZE (1024) DWORD code; - wchar_t msg[BUFFER_SIZE]; - SQLInstallerError(1, &code, msg, BUFFER_SIZE, NULL); - - std::wstringstream buf; - buf << L"Message: \"" << msg << L"\", Code: " << code; - std::wstring error_msg = buf.str(); - - PostError(code, const_cast(error_msg.c_str())); + std::vector msg(BUFFER_SIZE); + SQLInstallerError(1, &code, msg.data(), BUFFER_SIZE, NULL); + +#ifdef __linux__ + std::string code_str = std::to_string(code); + std::u16string code_u16 = arrow::util::UTF8StringToUTF16(code_str).ValueOr( + u"unknown code. Error during utf8 to utf16 conversion"); + std::u16string error_msg = u"Message: \"" + + std::u16string(reinterpret_cast(msg.data())) + + u"\", Code: " + code_u16; +#else + // Windows/macOS + std::wstring error_msg = + L"Message: \"" + std::wstring(msg.data()) + L"\", Code: " + std::to_wstring(code); +#endif // __linux__ + + PostError(code, const_cast(reinterpret_cast(error_msg.c_str()))); } /** @@ -62,7 +94,7 @@ void PostLastInstallerError() { * @return True on success and false on fail. */ bool UnregisterDsn(const std::wstring& dsn) { - if (SQLRemoveDSNFromIni(dsn.c_str())) { + if (SQLRemoveDSNFromIni(GET_SQWCHAR_PTR(dsn))) { return true; } @@ -79,14 +111,9 @@ bool UnregisterDsn(const std::wstring& dsn) { */ bool RegisterDsn(const Configuration& config, LPCWSTR driver) { const std::string& dsn = config.Get(FlightSqlConnection::DSN); - auto wdsn_result = arrow::util::UTF8ToWideString(dsn); - if (!wdsn_result.status().ok()) { - PostArrowUtilError(wdsn_result.status()); - return false; - } - std::wstring wdsn = wdsn_result.ValueOrDie(); + CONVERT_UTF8_TO_SQLWCHAR_OR_RETURN(wdsn, dsn); - if (!SQLWriteDSNToIni(wdsn.c_str(), driver)) { + if (!SQLWriteDSNToIni(reinterpret_cast(wdsn.c_str()), driver)) { PostLastInstallerError(); return false; } @@ -99,22 +126,13 @@ bool RegisterDsn(const Configuration& config, LPCWSTR driver) { continue; } - auto wkey_result = arrow::util::UTF8ToWideString(key); - if (!wkey_result.status().ok()) { - PostArrowUtilError(wkey_result.status()); - return false; - } - std::wstring wkey = wkey_result.ValueOrDie(); - - auto wvalue_result = arrow::util::UTF8ToWideString(it->second); - if (!wvalue_result.status().ok()) { - PostArrowUtilError(wvalue_result.status()); - return false; - } - std::wstring wvalue = wvalue_result.ValueOrDie(); + CONVERT_UTF8_TO_SQLWCHAR_OR_RETURN(wkey, key); + CONVERT_UTF8_TO_SQLWCHAR_OR_RETURN(wvalue, it->second); - if (!SQLWritePrivateProfileString(wdsn.c_str(), wkey.c_str(), wvalue.c_str(), - L"ODBC.INI")) { + if (!SQLWritePrivateProfileString(reinterpret_cast(wdsn.c_str()), + reinterpret_cast(wkey.c_str()), + reinterpret_cast(wvalue.c_str()), + ODBC_INI)) { PostLastInstallerError(); return false; } 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 fa0a35274a8e..fe6a2d4d7c38 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/util.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/util.cc @@ -136,6 +136,7 @@ SqlDataType GetDataTypeFromArrowFieldV3(const std::shared_ptr& field, case Type::LARGE_LIST: case Type::MAX_ID: case Type::NA: + default: break; } @@ -803,6 +804,8 @@ std::shared_ptr GetDefaultDataTypeForTypeId(Type::type type_id) { return arrow::time64(TimeUnit::MICRO); case Type::TIMESTAMP: return arrow::timestamp(TimeUnit::SECOND); + default: + break; } throw DriverException(std::string("Invalid type id: ") + std::to_string(type_id)); 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 f513c8d340dd..63ce6d6549d6 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/util.h +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/util.h @@ -29,6 +29,15 @@ #include #include +#ifdef __linux__ +// Linux driver manager uses utf16string +# define CONVERT_SQLWCHAR_STR(wvar, var) \ + CONVERT_UTF16_STR(const std::u16string wvar, var) +#else +// Windows and macOS uses wstring +# define CONVERT_SQLWCHAR_STR(wvar, var) CONVERT_WIDE_STR(const std::wstring wvar, var) +#endif // __linux__ + #define CONVERT_WIDE_STR(wstring_var, utf8_target) \ wstring_var = [&] { \ arrow::Result res = arrow::util::UTF8ToWideString(utf8_target); \ @@ -43,6 +52,13 @@ return res.ValueOrDie(); \ }() +#define CONVERT_UTF16_STR(utf16string_var, utf8_target) \ + utf16string_var = [&] { \ + arrow::Result res = arrow::util::UTF8StringToUTF16(utf8_target); \ + arrow::flight::sql::odbc::util::ThrowIfNotOK(res.status()); \ + return res.ValueOrDie(); \ + }() + namespace arrow::flight::sql::odbc { namespace util { 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 307b4812517a..640022429798 100644 --- a/cpp/src/arrow/flight/sql/odbc/tests/errors_test.cc +++ b/cpp/src/arrow/flight/sql/odbc/tests/errors_test.cc @@ -494,7 +494,6 @@ TYPED_TEST(ErrorsOdbcV2Test, TestSQLErrorEnvErrorFromDriverManager) { EXPECT_FALSE(std::wstring(message).empty()); } -// TODO: verify that `SQLGetConnectOption` is not required by Excel. #ifndef __APPLE__ TYPED_TEST(ErrorsOdbcV2Test, TestSQLErrorConnError) { // Test ODBC 2.0 API SQLError with ODBC ver 2. diff --git a/cpp/src/arrow/symbols.map b/cpp/src/arrow/symbols.map index d3c38c22c907..397bd80060a9 100644 --- a/cpp/src/arrow/symbols.map +++ b/cpp/src/arrow/symbols.map @@ -35,6 +35,8 @@ Arrow*; # ARROW-14771: export Protobuf symbol table descriptor_table_Flight*_2eproto; + # Export Arrow Flight SQL ODBC symbols + SQL*; # Symbols marked as 'local' are not exported by the DSO and thus may not # be used by client applications. Everything except the above falls here.