Skip to content

Commit 8b8c0d7

Browse files
justing-bqvic-tsangalinaliBQ
authored
Enable ODBC build in MacOS CI
Co-authored-by: Victor Tsang <victor.tsang@improving.com> Co-authored-by: Alina (Xi) Li <alina.li@improving.com>
1 parent 13002cf commit 8b8c0d7

18 files changed

Lines changed: 141 additions & 42 deletions

‎.github/workflows/cpp.yml‎

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,8 @@ jobs:
194194
ARROW_BUILD_TESTS: ON
195195
ARROW_DATASET: ON
196196
ARROW_FLIGHT: ON
197+
ARROW_FLIGHT_SQL: ON
198+
ARROW_FLIGHT_SQL_ODBC: ON
197199
ARROW_GANDIVA: ON
198200
ARROW_GCS: ON
199201
ARROW_HDFS: ON
@@ -230,6 +232,9 @@ jobs:
230232
brew uninstall pkg-config || :
231233
brew uninstall pkg-config@0.29.2 || :
232234
brew bundle --file=cpp/Brewfile
235+
export LIBIODBC_DIR="$(brew --cellar libiodbc)/$(brew list --versions libiodbc | awk '{print $2}')"
236+
echo ODBC_INCLUDE_DIR="$LIBIODBC_DIR/include" >> $GITHUB_ENV
237+
echo ODBC_LIB_DIR="$LIBIODBC_DIR/lib" >> $GITHUB_ENV
233238
- name: Install MinIO
234239
run: |
235240
$(brew --prefix bash)/bin/bash \
@@ -257,20 +262,20 @@ jobs:
257262
restore-keys: cpp-ccache-macos-${{ matrix.macos-version }}-
258263
- name: Build
259264
run: |
260-
if [ "${{ matrix.macos-version }}" = "15-intel" ]; then
261-
# This is a workaround.
262-
#
263-
# Homebrew uses /usr/local as prefix. So packages
264-
# installed by Homebrew also use /usr/local/include. We
265-
# want to include headers for packages installed by
266-
# Homebrew as system headers to ignore warnings in them.
267-
# But "-isystem /usr/local/include" isn't used by CMake
268-
# because /usr/local/include is marked as the default
269-
# include path. So we disable -Werror to avoid build error
270-
# by warnings from packages installed by Homebrew.
271-
export BUILD_WARNING_LEVEL=PRODUCTION
272-
fi
265+
# Homebrew uses /usr/local as prefix. So packages
266+
# installed by Homebrew also use /usr/local/include. We
267+
# want to include headers for packages installed by
268+
# Homebrew as system headers to ignore warnings in them.
269+
# But "-isystem /usr/local/include" isn't used by CMake
270+
# because /usr/local/include is marked as the default
271+
# include path. So we disable -Werror to avoid build error
272+
# by warnings from packages installed by Homebrew.
273+
export BUILD_WARNING_LEVEL=PRODUCTION
273274
ci/scripts/cpp_build.sh $(pwd) $(pwd)/build
275+
- name: Register Flight SQL ODBC Driver
276+
run: |
277+
chmod +x cpp/src/arrow/flight/sql/odbc/install/mac/install_odbc.sh
278+
sudo cpp/src/arrow/flight/sql/odbc/install/mac/install_odbc.sh $(pwd)/build/cpp/debug/libarrow_flight_sql_odbc.dylib
274279
- name: Test
275280
shell: bash
276281
run: |

‎ci/scripts/cpp_build.sh‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ else
260260
-DCMAKE_BUILD_TYPE=${ARROW_BUILD_TYPE:-debug} \
261261
-DCMAKE_VERBOSE_MAKEFILE=${CMAKE_VERBOSE_MAKEFILE:-OFF} \
262262
-DCMAKE_C_FLAGS="${CFLAGS:-}" \
263-
-DCMAKE_CXX_FLAGS="${CXXFLAGS:-}" \
263+
-DCMAKE_CXX_FLAGS="${CXXFLAGS:-} -I${ODBC_INCLUDE_DIR:-} -L${ODBC_LIB_DIR:-}" \
264264
-DCMAKE_CXX_STANDARD="${CMAKE_CXX_STANDARD:-17}" \
265265
-DCMAKE_INSTALL_LIBDIR=${CMAKE_INSTALL_LIBDIR:-lib} \
266266
-DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX:-${ARROW_HOME}} \
@@ -271,6 +271,7 @@ else
271271
-DgRPC_SOURCE=${gRPC_SOURCE:-} \
272272
-DGTest_SOURCE=${GTest_SOURCE:-} \
273273
-Dlz4_SOURCE=${lz4_SOURCE:-} \
274+
-DODBC_INCLUDE_DIR="${ODBC_INCLUDE_DIR:-}" \
274275
-Dopentelemetry-cpp_SOURCE=${opentelemetry_cpp_SOURCE:-} \
275276
-DORC_SOURCE=${ORC_SOURCE:-} \
276277
-DPARQUET_BUILD_EXAMPLES=${PARQUET_BUILD_EXAMPLES:-OFF} \

‎ci/scripts/cpp_test.sh‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ case "$(uname)" in
6161
n_jobs=$(sysctl -n hw.ncpu)
6262
# TODO: https://github.com/apache/arrow/issues/40410
6363
exclude_tests+=("arrow-s3fs-test")
64+
exclude_tests+=("arrow-flight-sql-odbc-test")
6465
;;
6566
MINGW*)
6667
n_jobs=${NUMBER_OF_PROCESSORS:-1}

‎cpp/Brewfile‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ brew "git"
2828
brew "glog"
2929
brew "googletest"
3030
brew "grpc"
31+
brew "libiodbc"
3132
brew "llvm"
3233
brew "lz4"
3334
brew "mimalloc"

‎cpp/CMakePresets.json‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,17 @@
315315
"displayName": "Debug build with tests and Flight SQL",
316316
"cacheVariables": {}
317317
},
318+
{
319+
"name": "ninja-debug-flight-sql-odbc",
320+
"inherits": [
321+
"features-flight-sql",
322+
"base-debug"
323+
],
324+
"displayName": "Debug build with tests and Flight SQL ODBC",
325+
"cacheVariables": {
326+
"ARROW_FLIGHT_SQL_ODBC": "ON"
327+
}
328+
},
318329
{
319330
"name": "ninja-debug-gandiva",
320331
"inherits": [
@@ -504,6 +515,17 @@
504515
"displayName": "Release build with Flight SQL",
505516
"cacheVariables": {}
506517
},
518+
{
519+
"name": "ninja-release-flight-sql-odbc",
520+
"inherits": [
521+
"features-flight-sql",
522+
"base-release"
523+
],
524+
"displayName": "Release build with Flight SQL ODBC",
525+
"cacheVariables": {
526+
"ARROW_FLIGHT_SQL_ODBC": "ON"
527+
}
528+
},
507529
{
508530
"name": "ninja-release-gandiva",
509531
"inherits": [

‎cpp/cmake_modules/DefineOptions.cmake‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,8 @@ macro(tsort_bool_option_dependencies)
107107
endmacro()
108108

109109
macro(resolve_option_dependencies)
110-
# Arrow Flight SQL ODBC is available only for Windows for now.
111-
if(NOT WIN32)
110+
# Arrow Flight SQL ODBC is available only for Windows and macOS for now.
111+
if(NOT WIN32 AND NOT APPLE)
112112
set(ARROW_FLIGHT_SQL_ODBC OFF)
113113
endif()
114114
if(MSVC_TOOLCHAIN)

‎cpp/src/arrow/flight/sql/odbc/entry_points.cc‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,3 +317,9 @@ SQLRETURN SQL_API SQLDescribeCol(SQLHSTMT stmt, SQLUSMALLINT column_number,
317317
stmt, column_number, column_name, buffer_length, name_length_ptr, data_type_ptr,
318318
column_size_ptr, decimal_digits_ptr, nullable_ptr);
319319
}
320+
321+
SQLRETURN SQL_API SQLGetConnectOption(SQLHDBC ConnectionHandle, SQLUSMALLINT Option,
322+
SQLPOINTER Value) {
323+
// TODO implement ODBC2 APIs
324+
return SQL_ERROR;
325+
}

‎cpp/src/arrow/flight/sql/odbc/install/mac/install_odbc.sh‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ USER_ODBCINST_FILE="$HOME/Library/ODBC/odbcinst.ini"
3636
DRIVER_NAME="Apache Arrow Flight SQL ODBC Driver"
3737
DSN_NAME="Apache Arrow Flight SQL ODBC DSN"
3838

39+
mkdir -p $HOME/Library/ODBC
40+
3941
touch "$USER_ODBCINST_FILE"
4042

4143
# Admin privilege is needed to add ODBC driver registration

‎cpp/src/arrow/flight/sql/odbc/odbc_api.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -853,7 +853,7 @@ SQLRETURN SQLDriverConnect(SQLHDBC conn, SQLHWND window_handle,
853853
}
854854
#else
855855
// Attempt connection without loading DSN window on macOS/Linux
856-
connection->Connect(dsn, properties, missing_properties);
856+
connection->Connect(dsn_value, properties, missing_properties);
857857
#endif
858858
// Copy connection string to out_connection_string after connection attempt
859859
return ODBC::GetStringAttribute(true, connection_string, false, out_connection_string,

‎cpp/src/arrow/flight/sql/odbc/odbc_impl/odbc_handle.h‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,18 @@ class ODBCHandle {
4747
try {
4848
GetDiagnostics().Clear();
4949
rc = function();
50-
} catch (const arrow::flight::sql::odbc::DriverException& ex) {
50+
} catch (const arrow::flight::sql::odbc::AuthenticationException& ex) {
51+
GetDiagnostics().AddError(arrow::flight::sql::odbc::DriverException(
52+
ex.GetMessageText(), ex.GetSqlState(), ex.GetNativeError()));
53+
} catch (const arrow::flight::sql::odbc::NullWithoutIndicatorException& ex) {
54+
GetDiagnostics().AddError(arrow::flight::sql::odbc::DriverException(
55+
ex.GetMessageText(), ex.GetSqlState(), ex.GetNativeError()));
56+
}
57+
// on mac, DriverException doesn't catch the subclass exceptions hence we added
58+
// the following above.
59+
// GH-48278 TODO investigate if there is a way to catch all the subclass exceptions under
60+
// DriverException
61+
catch (const arrow::flight::sql::odbc::DriverException& ex) {
5162
GetDiagnostics().AddError(ex);
5263
} catch (const std::bad_alloc&) {
5364
GetDiagnostics().AddError(arrow::flight::sql::odbc::DriverException(

0 commit comments

Comments
 (0)