Skip to content

GH-46004: [C++][FlightRPC] Enable ODBC Build In C++ Workflows #47689

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+82 −13
Open

GH-46004: [C++][FlightRPC] Enable ODBC Build In C++ Workflows #47689

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .github/workflows/cpp.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,10 @@ jobs:
PIPX_BASE_PYTHON: ${{ steps.python-install.outputs.python-path }}
run: |
ci/scripts/install_gcs_testbench.sh default
- 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%\libarrow_flight_sql_odbc.dll
- name: Test
shell: msys2 {0}
run: |
Expand Down
2 changes: 2 additions & 0 deletions ci/scripts/cpp_build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ if [ "${ARROW_ENABLE_THREADING:-ON}" = "OFF" ]; then
ARROW_AZURE=OFF
ARROW_FLIGHT=OFF
ARROW_FLIGHT_SQL=OFF
ARROW_FLIGHT_SQL_ODBC=OFF
ARROW_GCS=OFF
ARROW_JEMALLOC=OFF
ARROW_MIMALLOC=OFF
Expand Down Expand Up @@ -212,6 +213,7 @@ else
-DARROW_FILESYSTEM=${ARROW_FILESYSTEM:-ON} \
-DARROW_FLIGHT=${ARROW_FLIGHT:-OFF} \
-DARROW_FLIGHT_SQL=${ARROW_FLIGHT_SQL:-OFF} \
-DARROW_FLIGHT_SQL_ODBC=${ARROW_FLIGHT_SQL_ODBC:-OFF} \
-DARROW_FUZZING=${ARROW_FUZZING:-OFF} \
-DARROW_GANDIVA_PC_CXX_FLAGS=${ARROW_GANDIVA_PC_CXX_FLAGS:-} \
-DARROW_GANDIVA=${ARROW_GANDIVA:-OFF} \
Expand Down
2 changes: 1 addition & 1 deletion cpp/cmake_modules/DefineOptions.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ endmacro()

macro(resolve_option_dependencies)
# Arrow Flight SQL ODBC is available only for Windows for now.
if(NOT MSVC_TOOLCHAIN)
if(NOT WIN32)
set(ARROW_FLIGHT_SQL_ODBC OFF)
endif()
if(MSVC_TOOLCHAIN)
Expand Down
2 changes: 1 addition & 1 deletion cpp/cmake_modules/ThirdpartyToolchain.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1269,7 +1269,7 @@ if(ARROW_USE_BOOST)
endif()
if(ARROW_BOOST_REQUIRE_LIBRARY)
set(ARROW_BOOST_COMPONENTS filesystem system)
if(ARROW_FLIGHT_SQL_ODBC AND MSVC)
if(ARROW_FLIGHT_SQL_ODBC)
list(APPEND ARROW_BOOST_COMPONENTS locale)
endif()
if(ARROW_ENABLE_THREADING)
Expand Down
8 changes: 4 additions & 4 deletions cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/string_array_accessor.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
namespace arrow::flight::sql::odbc {
namespace {

#if defined _WIN32 || defined _WIN64
#if defined _WIN32
std::string Utf8ToCLocale(const char* utf8_str, int len) {
thread_local boost::locale::generator g;
g.locale_cache_enabled(true);
Expand All @@ -36,7 +36,7 @@ std::string Utf8ToCLocale(const char* utf8_str, int len) {
template <typename CHAR_TYPE>
inline RowStatus MoveSingleCellToCharBuffer(
std::vector<uint8_t>& buffer, int64_t& last_retrieved_arrow_row,
#if defined _WIN32 || defined _WIN64
#if defined _WIN32
std::string& clocale_str,
#endif
ColumnBinding* binding, StringArray* array, int64_t arrow_row, int64_t i,
Expand All @@ -57,7 +57,7 @@ inline RowStatus MoveSingleCellToCharBuffer(
value = buffer.data();
size_in_bytes = buffer.size();
} else {
#if defined _WIN32 || defined _WIN64
#if defined _WIN32
// Convert to C locale string
if (last_retrieved_arrow_row != arrow_row) {
clocale_str = Utf8ToCLocale(raw_value, raw_value_length);
Expand Down Expand Up @@ -124,7 +124,7 @@ RowStatus StringArrayFlightSqlAccessor<TARGET_TYPE, CHAR_TYPE>::MoveSingleCellIm
ColumnBinding* binding, int64_t arrow_row, int64_t i, int64_t& value_offset,
bool update_value_offset, Diagnostics& diagnostics) {
return MoveSingleCellToCharBuffer<CHAR_TYPE>(buffer_, last_arrow_row_,
#if defined _WIN32 || defined _WIN64
#if defined _WIN32
clocale_str_,
#endif
binding, this->GetArray(), arrow_row, i,
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/string_array_accessor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class StringArrayFlightSqlAccessor

private:
std::vector<uint8_t> buffer_;
#if defined _WIN32 || defined _WIN64
#if defined _WIN32
std::string clocale_str_;
#endif
int64_t last_arrow_row_;
Expand Down
1 change: 0 additions & 1 deletion cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/string_array_accessor_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,6 @@ TEST(StringArrayAccessor, Test_CDataType_WCHAR_Truncation) {
ColumnBinding binding(CDataType_WCHAR, 0, 0, buffer.data(), max_str_len,
str_len_buffer.data());

std::basic_stringstream<uint8_t> ss;
int64_t value_offset = 0;

// Construct the whole string by concatenating smaller chunks from
Expand Down
1 change: 1 addition & 0 deletions cpp/src/arrow/flight/sql/odbc/odbc_impl/address_info.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "arrow/flight/sql/odbc/odbc_impl/platform.h"

#include <sys/types.h>
#include <cstdint>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was this missing previously?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think so. I recall it was causing build issues on workflows. The build works with or without this header on my local machine (Windows MSVC), but the CI builds can have problems without the header

#if !_WIN32
# include <netdb.h>
#endif
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_connection.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ boost::optional<int32_t> FlightSqlConnection::GetStringColumnLength(
}

bool FlightSqlConnection::GetUseWideChar(const ConnPropertyMap& conn_property_map) {
#if defined _WIN32 || defined _WIN64
#if defined _WIN32
// Windows should use wide chars by default
bool default_value = true;
#else
Expand Down
2 changes: 2 additions & 0 deletions cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_statement_get_columns.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
// specific language governing permissions and limitations
// under the License.

#pragma once

#include <optional>
#include "arrow/array/builder_binary.h"
#include "arrow/array/builder_primitive.h"
Expand Down
2 changes: 2 additions & 0 deletions cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_statement_get_type_info.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
// specific language governing permissions and limitations
// under the License.

#pragma once

#include <optional>
#include "arrow/array/builder_binary.h"
#include "arrow/array/builder_primitive.h"
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/flight/sql/odbc/odbc_impl/system_trust_store.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

#include "arrow/flight/sql/odbc/odbc_impl/system_trust_store.h"

#if defined _WIN32 || defined _WIN64
#if defined _WIN32

namespace arrow::flight::sql::odbc {
bool SystemTrustStore::HasNext() {
Expand Down
6 changes: 5 additions & 1 deletion cpp/src/arrow/flight/sql/odbc/odbc_impl/system_trust_store.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,17 @@

#pragma once

#if defined _WIN32 || defined _WIN64
#if defined _WIN32

# include <windows.h>

# include <wincrypt.h>

# include <bcrypt.h>

// prsht.h needs to be included before cryptuiapi.h to avoid build conflict
# include <prsht.h>

# include <cryptuiapi.h>

# include <tchar.h>
Expand Down
6 changes: 4 additions & 2 deletions cpp/src/arrow/flight/sql/odbc/odbc_impl/ui/custom_window.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
// before Windowsx.h and commctrl.h
#include "arrow/flight/sql/odbc/odbc_impl/platform.h"

#include "arrow/util/logging.h"

#include <Windowsx.h>
#include <commctrl.h>

Expand Down Expand Up @@ -52,7 +54,7 @@ LRESULT CALLBACK CustomWindow::WndProc(HWND hwnd, UINT msg, WPARAM wparam,

switch (msg) {
case WM_NCCREATE: {
_ASSERT(lparam != NULL);
ARROW_DCHECK_NE(lparam, NULL);

CREATESTRUCT* create_struct = reinterpret_cast<CREATESTRUCT*>(lparam);

Expand All @@ -64,7 +66,7 @@ LRESULT CALLBACK CustomWindow::WndProc(HWND hwnd, UINT msg, WPARAM wparam,
}

case WM_CREATE: {
_ASSERT(window != NULL);
ARROW_DCHECK_NE(window, NULL);

window->SetHandle(hwnd);

Expand Down
53 changes: 53 additions & 0 deletions cpp/src/arrow/flight/sql/odbc/tests/install_odbc.cmd
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need _amd64 in file name?

It seems that this doesn't have any amd64 specific part.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea that's fair, I have renamed to install_odbc

Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
@REM Licensed to the Apache Software Foundation (ASF) under one
@REM or more contributor license agreements. See the NOTICE file
@REM distributed with this work for additional information
@REM regarding copyright ownership. The ASF licenses this file
@REM to you under the Apache License, Version 2.0 (the
@REM "License"); you may not use this file except in compliance
@REM with the License. You may obtain a copy of the License at
@REM
@REM http://www.apache.org/licenses/LICENSE-2.0
@REM
@REM Unless required by applicable law or agreed to in writing,
@REM software distributed under the License is distributed on an
@REM "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
@REM KIND, either express or implied. See the License for the
@REM specific language governing permissions and limitations
@REM under the License.

@echo off

set ODBC_64BIT=%1

@REM enable delayed variable expansion to make environment variables enclosed with "!" to be evaluated
@REM when the command is executed instead of when the command is parsed
setlocal enableextensions enabledelayedexpansion

if [%ODBC_64BIT%] == [] (
echo error: 64-bit driver is not specified. Call format: install_odbc abs_path_to_64_bit_driver
pause
exit /b 1
)

if exist %ODBC_64BIT% (
for %%i IN (%ODBC_64BIT%) DO IF EXIST %%~si\NUL (
echo warning: The path you have specified seems to be a directory. Note that you have to specify path to driver file itself instead.
)
echo Installing 64-bit driver: %ODBC_64BIT%
reg add "HKEY_LOCAL_MACHINE\SOFTWARE\ODBC\ODBCINST.INI\Apache Arrow Flight SQL ODBC Driver" /v DriverODBCVer /t REG_SZ /d "03.80" /f
reg add "HKEY_LOCAL_MACHINE\SOFTWARE\ODBC\ODBCINST.INI\Apache Arrow Flight SQL ODBC Driver" /v UsageCount /t REG_DWORD /d 00000001 /f
reg add "HKEY_LOCAL_MACHINE\SOFTWARE\ODBC\ODBCINST.INI\Apache Arrow Flight SQL ODBC Driver" /v Driver /t REG_SZ /d %ODBC_64BIT% /f
reg add "HKEY_LOCAL_MACHINE\SOFTWARE\ODBC\ODBCINST.INI\Apache Arrow Flight SQL ODBC Driver" /v Setup /t REG_SZ /d %ODBC_64BIT% /f
reg add "HKEY_LOCAL_MACHINE\SOFTWARE\ODBC\ODBCINST.INI\ODBC Drivers" /v "Apache Arrow Flight SQL ODBC Driver" /t REG_SZ /d "Installed" /f

if !ERRORLEVEL! NEQ 0 (
echo Error occurred while registering 64-bit driver. Exiting.
echo ERRORLEVEL: !ERRORLEVEL!
exit !ERRORLEVEL!
)
) else (
echo 64-bit driver can not be found: %ODBC_64BIT%
echo Call format: install_odbc abs_path_to_64_bit_driver
pause
exit /b 1
)
Loading