Enable ODBC build in MacOS CI

.github/workflows/cpp.yml

@@ -194,6 +194,8 @@ jobs:
       ARROW_BUILD_TESTS: ON
       ARROW_DATASET: ON
       ARROW_FLIGHT: ON
+      ARROW_FLIGHT_SQL: ON
+      ARROW_FLIGHT_SQL_ODBC: ON
       ARROW_GANDIVA: ON
       ARROW_GCS: ON
       ARROW_HDFS: ON
@@ -230,6 +232,9 @@ jobs:
           brew uninstall pkg-config || :
           brew uninstall pkg-config@0.29.2 || :
           brew bundle --file=cpp/Brewfile
+          export LIBIODBC_DIR="$(brew --cellar libiodbc)/$(brew list --versions libiodbc | awk '{print $2}')"
+          echo ODBC_INCLUDE_DIR="$LIBIODBC_DIR/include" >> $GITHUB_ENV
+          echo ODBC_LIB_DIR="$LIBIODBC_DIR/lib" >> $GITHUB_ENV
       - name: Install MinIO
         run: |
           $(brew --prefix bash)/bin/bash \
@@ -257,20 +262,20 @@ jobs:
           restore-keys: cpp-ccache-macos-${{ matrix.macos-version }}-
       - name: Build
         run: |
-          if [ "${{ matrix.macos-version }}" = "15-intel" ]; then
-            # This is a workaround.
-            #
-            # Homebrew uses /usr/local as prefix. So packages
-            # installed by Homebrew also use /usr/local/include. We
-            # want to include headers for packages installed by
-            # Homebrew as system headers to ignore warnings in them.
-            # But "-isystem /usr/local/include" isn't used by CMake
-            # because /usr/local/include is marked as the default
-            # include path. So we disable -Werror to avoid build error
-            # by warnings from packages installed by Homebrew.
-            export BUILD_WARNING_LEVEL=PRODUCTION
-          fi
+          # Homebrew uses /usr/local as prefix. So packages
+          # installed by Homebrew also use /usr/local/include. We
+          # want to include headers for packages installed by
+          # Homebrew as system headers to ignore warnings in them.
+          # But "-isystem /usr/local/include" isn't used by CMake
+          # because /usr/local/include is marked as the default
+          # include path. So we disable -Werror to avoid build error
+          # by warnings from packages installed by Homebrew.
+          export BUILD_WARNING_LEVEL=PRODUCTION
           ci/scripts/cpp_build.sh $(pwd) $(pwd)/build
+      - name: Register Flight SQL ODBC Driver
+        run: |
+           chmod +x cpp/src/arrow/flight/sql/odbc/install/mac/install_odbc.sh
+           sudo cpp/src/arrow/flight/sql/odbc/install/mac/install_odbc.sh $(pwd)/build/cpp/debug/libarrow_flight_sql_odbc.dylib
       - name: Test
         shell: bash
         run: |

ci/scripts/cpp_build.sh

@@ -260,7 +260,7 @@ else
     -DCMAKE_BUILD_TYPE=${ARROW_BUILD_TYPE:-debug} \
     -DCMAKE_VERBOSE_MAKEFILE=${CMAKE_VERBOSE_MAKEFILE:-OFF} \
     -DCMAKE_C_FLAGS="${CFLAGS:-}" \
-    -DCMAKE_CXX_FLAGS="${CXXFLAGS:-}" \
+    -DCMAKE_CXX_FLAGS="${CXXFLAGS:-} -I${ODBC_INCLUDE_DIR:-} -L${ODBC_LIB_DIR:-}" \
     -DCMAKE_CXX_STANDARD="${CMAKE_CXX_STANDARD:-17}" \
     -DCMAKE_INSTALL_LIBDIR=${CMAKE_INSTALL_LIBDIR:-lib} \
     -DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX:-${ARROW_HOME}} \
@@ -271,6 +271,7 @@ else
     -DgRPC_SOURCE=${gRPC_SOURCE:-} \
     -DGTest_SOURCE=${GTest_SOURCE:-} \
     -Dlz4_SOURCE=${lz4_SOURCE:-} \
+    -DODBC_INCLUDE_DIR="${ODBC_INCLUDE_DIR:-}" \
     -Dopentelemetry-cpp_SOURCE=${opentelemetry_cpp_SOURCE:-} \
     -DORC_SOURCE=${ORC_SOURCE:-} \
     -DPARQUET_BUILD_EXAMPLES=${PARQUET_BUILD_EXAMPLES:-OFF} \

ci/scripts/cpp_test.sh

@@ -61,6 +61,7 @@ case "$(uname)" in
     n_jobs=$(sysctl -n hw.ncpu)
     # TODO: https://github.com/apache/arrow/issues/40410
     exclude_tests+=("arrow-s3fs-test")
+    exclude_tests+=("arrow-flight-sql-odbc-test")
     ;;
   MINGW*)
     n_jobs=${NUMBER_OF_PROCESSORS:-1}

cpp/Brewfile

@@ -28,6 +28,7 @@ brew "git"
 brew "glog"
 brew "googletest"
 brew "grpc"
+brew "libiodbc"
 brew "llvm"
 brew "lz4"
 brew "mimalloc"

cpp/CMakePresets.json

@@ -315,6 +315,17 @@
       "displayName": "Debug build with tests and Flight SQL",
       "cacheVariables": {}
     },
+    {
+      "name": "ninja-debug-flight-sql-odbc",
+      "inherits": [
+        "features-flight-sql",
+        "base-debug"
+      ],
+      "displayName": "Debug build with tests and Flight SQL ODBC",
+      "cacheVariables": {
+        "ARROW_FLIGHT_SQL_ODBC": "ON"
+      }
+    },
     {
       "name": "ninja-debug-gandiva",
       "inherits": [
@@ -504,6 +515,17 @@
       "displayName": "Release build with Flight SQL",
       "cacheVariables": {}
     },
+    {
+      "name": "ninja-release-flight-sql-odbc",
+      "inherits": [
+        "features-flight-sql",
+        "base-release"
+      ],
+      "displayName": "Release build with Flight SQL ODBC",
+      "cacheVariables": {
+        "ARROW_FLIGHT_SQL_ODBC": "ON"
+      }
+    },
     {
       "name": "ninja-release-gandiva",
       "inherits": [

cpp/cmake_modules/DefineOptions.cmake

@@ -107,8 +107,8 @@ macro(tsort_bool_option_dependencies)
 endmacro()
 
 macro(resolve_option_dependencies)
-  # Arrow Flight SQL ODBC is available only for Windows for now.
-  if(NOT WIN32)
+  # 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)

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

@@ -317,3 +317,9 @@ SQLRETURN SQL_API SQLDescribeCol(SQLHSTMT stmt, SQLUSMALLINT column_number,
       stmt, column_number, column_name, buffer_length, name_length_ptr, data_type_ptr,
       column_size_ptr, decimal_digits_ptr, nullable_ptr);
 }
+
+SQLRETURN SQL_API SQLGetConnectOption(SQLHDBC ConnectionHandle, SQLUSMALLINT Option,
+                                      SQLPOINTER Value) {
+  // TODO implement ODBC2 APIs
+  return SQL_ERROR;
+}

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

@@ -36,6 +36,8 @@ USER_ODBCINST_FILE="$HOME/Library/ODBC/odbcinst.ini"
 DRIVER_NAME="Apache Arrow Flight SQL ODBC Driver"
 DSN_NAME="Apache Arrow Flight SQL ODBC DSN"
 
+mkdir -p $HOME/Library/ODBC
+
 touch "$USER_ODBCINST_FILE"
 
 # Admin privilege is needed to add ODBC driver registration

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

@@ -853,7 +853,7 @@ SQLRETURN SQLDriverConnect(SQLHDBC conn, SQLHWND window_handle,
     }
 #else
     // Attempt connection without loading DSN window on macOS/Linux
-    connection->Connect(dsn, properties, missing_properties);
+    connection->Connect(dsn_value, properties, missing_properties);
 #endif
     // Copy connection string to out_connection_string after connection attempt
     return ODBC::GetStringAttribute(true, connection_string, false, out_connection_string,

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

@@ -47,7 +47,18 @@ class ODBCHandle {
     try {
       GetDiagnostics().Clear();
       rc = function();
-    } catch (const arrow::flight::sql::odbc::DriverException& ex) {
+    } catch (const arrow::flight::sql::odbc::AuthenticationException& ex) {
+      GetDiagnostics().AddError(arrow::flight::sql::odbc::DriverException(
+          ex.GetMessageText(), ex.GetSqlState(), ex.GetNativeError()));
+    } catch (const arrow::flight::sql::odbc::NullWithoutIndicatorException& ex) {
+      GetDiagnostics().AddError(arrow::flight::sql::odbc::DriverException(
+          ex.GetMessageText(), ex.GetSqlState(), ex.GetNativeError()));
+    }
+    // on mac, DriverException doesn't catch the subclass exceptions hence we added
+    // the following above.
+    // GH-48278 TODO investigate if there is a way to catch all the subclass exceptions under
+    // DriverException
+    catch (const arrow::flight::sql::odbc::DriverException& ex) {
       GetDiagnostics().AddError(ex);
     } catch (const std::bad_alloc&) {
       GetDiagnostics().AddError(arrow::flight::sql::odbc::DriverException(

cpp/src/arrow/flight/sql/odbc/tests/CMakeLists.txt

@@ -17,7 +17,11 @@
 
 add_custom_target(tests)
 
-include_directories(${ODBC_INCLUDE_DIRS})
+if(WIN32)
+    include_directories(${ODBC_INCLUDE_DIRS})
+else()
+    include_directories(${ODBC_INCLUDE_DIR})
+endif()
 
 find_package(SQLite3Alt REQUIRED)
 

cpp/src/arrow/flight/sql/odbc/tests/columns_test.cc

@@ -398,7 +398,7 @@ TYPED_TEST(ColumnsTest, SQLColumnsTestInputData) {
   ValidateFetch(this->stmt, SQL_SUCCESS);
 }
 
-TEST_F(ColumnsMockTest, TestSQLColumnsAllColumns) {
+TEST_F(ColumnsMockTest, TestSQLColumnsAllColumnsSegFault) {
   // Check table pattern and column pattern returns all columns
 
   // Attempt to get all columns
@@ -1101,7 +1101,7 @@ TEST_F(ColumnsOdbcV2RemoteTest, TestSQLColumnsAllTypes) {
   EXPECT_EQ(SQL_NO_DATA, SQLFetch(this->stmt));
 }
 
-TEST_F(ColumnsMockTest, TestSQLColumnscolumn_pattern) {
+TEST_F(ColumnsMockTest, TestSQLColumnsColumnPatternSegFault) {
   // Checks filtering table with column name pattern.
   // Only check table and column name
 
@@ -2167,7 +2167,7 @@ TYPED_TEST(ColumnsOdbcV2Test, TestSQLColAttributesUpdatable) {
   ASSERT_EQ(SQL_ATTR_READWRITE_UNKNOWN, value);
 }
 
-TEST_F(ColumnsMockTest, SQLDescribeColValidateInput) {
+TEST_F(ColumnsMockTest, TestSQLDescribeColValidateInput) {
   this->CreateTestTables();
 
   SQLWCHAR sql_query[] = L"SELECT * FROM TestTable LIMIT 1;";
@@ -2176,7 +2176,7 @@ TEST_F(ColumnsMockTest, SQLDescribeColValidateInput) {
   SQLUSMALLINT bookmark_column = 0;
   SQLUSMALLINT out_of_range_column = 4;
   SQLUSMALLINT negative_column = -1;
-  SQLWCHAR column_name[1024];
+  SQLWCHAR column_name[1024] = {0};
   SQLSMALLINT buf_char_len =
       static_cast<SQLSMALLINT>(sizeof(column_name) / ODBC::GetSqlWCharSize());
   SQLSMALLINT name_length = 0;
@@ -2193,7 +2193,12 @@ TEST_F(ColumnsMockTest, SQLDescribeColValidateInput) {
   EXPECT_EQ(SQL_ERROR, SQLDescribeCol(this->stmt, bookmark_column, column_name,
                                       buf_char_len, &name_length, &data_type,
                                       &column_size, &decimal_digits, &nullable));
+#ifdef __APPLE__
+  // non-standard odbc error code for invalid column index
+  VerifyOdbcErrorState(SQL_HANDLE_STMT, this->stmt, kErrorStateS1002);
+#else
   VerifyOdbcErrorState(SQL_HANDLE_STMT, this->stmt, kErrorState07009);
+#endif // __APPLE__
 
   // Invalid descriptor index - index out of range
   EXPECT_EQ(SQL_ERROR, SQLDescribeCol(this->stmt, out_of_range_column, column_name,

cpp/src/arrow/flight/sql/odbc/tests/connection_test.cc

@@ -442,7 +442,7 @@ TEST_F(ConnectionRemoteTest, TestSQLDriverConnectInvalidUid) {
                        arrow::util::UTF8ToWideString(connect_str));
   std::vector<SQLWCHAR> connect_str0(wconnect_str.begin(), wconnect_str.end());
 
-  SQLWCHAR out_str[kOdbcBufferSize];
+  SQLWCHAR out_str[kOdbcBufferSize] = {0};
   SQLSMALLINT out_str_len;
 
   // Connecting to ODBC server.

cpp/src/arrow/flight/sql/odbc/tests/errors_test.cc

@@ -90,9 +90,12 @@ TYPED_TEST(ErrorsHandleTest, TestSQLGetDiagFieldWForConnectFailure) {
   SQLWCHAR message_text[kOdbcBufferSize];
   SQLSMALLINT message_text_length;
 
-  EXPECT_EQ(SQL_SUCCESS,
-            SQLGetDiagField(SQL_HANDLE_DBC, this->conn, RECORD_1, SQL_DIAG_MESSAGE_TEXT,
-                            message_text, kOdbcBufferSize, &message_text_length));
+  SQLRETURN ret =
+      SQLGetDiagField(SQL_HANDLE_DBC, this->conn, RECORD_1, SQL_DIAG_MESSAGE_TEXT,
+                      message_text, kOdbcBufferSize, &message_text_length);
+
+  // dependent on the size of the message it could output SQL_SUCCESS_WITH_INFO
+  EXPECT_TRUE(ret == SQL_SUCCESS || ret == SQL_SUCCESS_WITH_INFO);
 
   EXPECT_GT(message_text_length, 100);
 
@@ -304,10 +307,15 @@ TYPED_TEST(ErrorsTest, TestSQLGetDiagRecInputData) {
   EXPECT_EQ(SQL_NO_DATA, SQLGetDiagRec(SQL_HANDLE_DBC, this->conn, 1, 0, 0, 0, 0, 0));
 
   // Invalid handle
+#ifdef __APPLE__
+  // MacOS ODBC driver manager requires connection handle 
+  EXPECT_EQ(SQL_INVALID_HANDLE, SQLGetDiagRec(0, this->conn, 1, 0, 0, 0, 0, 0));
+#else
   EXPECT_EQ(SQL_INVALID_HANDLE, SQLGetDiagRec(0, 0, 0, 0, 0, 0, 0, 0));
+#endif // __APPLE__
 }
 
-TYPED_TEST(ErrorsTest, TestSQLErrorInputData) {
+TYPED_TEST(ErrorsOdbcV2Test, TestSQLErrorInputData) {
   // Test ODBC 2.0 API SQLError. Driver manager maps SQLError to SQLGetDiagRec.
   // SQLError does not post diagnostic records for itself.
 
@@ -316,7 +324,11 @@ TYPED_TEST(ErrorsTest, TestSQLErrorInputData) {
 
   EXPECT_EQ(SQL_NO_DATA, SQLError(0, this->conn, 0, 0, 0, 0, 0, 0));
 
+#ifdef __APPLE__
+  EXPECT_EQ(SQL_NO_DATA, SQLError(SQL_NULL_HENV, this->conn, this->stmt, 0, 0, 0, 0, 0));
+#else
   EXPECT_EQ(SQL_NO_DATA, SQLError(0, 0, this->stmt, 0, 0, 0, 0, 0));
+#endif // __APPLE__
 
   // Invalid handle
   EXPECT_EQ(SQL_INVALID_HANDLE, SQLError(0, 0, 0, 0, 0, 0, 0, 0));
@@ -391,8 +403,10 @@ TYPED_TEST(ErrorsTest, TestSQLErrorStmtError) {
   SQLINTEGER native_error = 0;
   SQLWCHAR message[SQL_MAX_MESSAGE_LENGTH] = {0};
   SQLSMALLINT message_length = 0;
-  ASSERT_EQ(SQL_SUCCESS, SQLError(0, 0, this->stmt, sql_state, &native_error, message,
-                                  SQL_MAX_MESSAGE_LENGTH, &message_length));
+  SQLRETURN ret = SQLError(0, this->conn, this->stmt, sql_state, &native_error, message,
+                           SQL_MAX_MESSAGE_LENGTH, &message_length);
+
+  EXPECT_TRUE(ret == SQL_SUCCESS || ret == SQL_SUCCESS_WITH_INFO);
 
   EXPECT_GT(message_length, 70);
 
@@ -426,8 +440,9 @@ TYPED_TEST(ErrorsTest, TestSQLErrorStmtWarning) {
   SQLINTEGER native_error = 0;
   SQLWCHAR message[SQL_MAX_MESSAGE_LENGTH] = {0};
   SQLSMALLINT message_length = 0;
-  ASSERT_EQ(SQL_SUCCESS, SQLError(0, 0, this->stmt, sql_state, &native_error, message,
-                                  SQL_MAX_MESSAGE_LENGTH, &message_length));
+  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, 50);
 
@@ -508,8 +523,8 @@ TYPED_TEST(ErrorsOdbcV2Test, TestSQLErrorStmtError) {
   SQLINTEGER native_error = 0;
   SQLWCHAR message[SQL_MAX_MESSAGE_LENGTH] = {0};
   SQLSMALLINT message_length = 0;
-  ASSERT_EQ(SQL_SUCCESS, SQLError(0, 0, this->stmt, sql_state, &native_error, message,
-                                  SQL_MAX_MESSAGE_LENGTH, &message_length));
+  ASSERT_EQ(SQL_SUCCESS, SQLError(0, this->conn, this->stmt, sql_state, &native_error,
+                                  message, SQL_MAX_MESSAGE_LENGTH, &message_length));
 
   EXPECT_GT(message_length, 70);
 
@@ -544,8 +559,8 @@ TYPED_TEST(ErrorsOdbcV2Test, TestSQLErrorStmtWarning) {
   SQLINTEGER native_error = 0;
   SQLWCHAR message[SQL_MAX_MESSAGE_LENGTH] = {0};
   SQLSMALLINT message_length = 0;
-  ASSERT_EQ(SQL_SUCCESS, SQLError(0, 0, this->stmt, sql_state, &native_error, message,
-                                  SQL_MAX_MESSAGE_LENGTH, &message_length));
+  ASSERT_EQ(SQL_SUCCESS, SQLError(0, this->conn, this->stmt, sql_state, &native_error,
+                                  message, SQL_MAX_MESSAGE_LENGTH, &message_length));
 
   EXPECT_GT(message_length, 50);
 

cpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.cc

@@ -130,9 +130,9 @@ std::wstring ODBCRemoteTestBase::GetQueryAllDataTypes() {
           CAST(true AS BOOLEAN) AS bit_true,
 
           --Character types
-          'Z' AS c_char, '你' AS c_wchar,
+          'Z' AS c_char, _utf8'你' AS c_wchar,
 
-          '你好' AS c_wvarchar,
+          _utf8'你好' AS c_wvarchar,
 
           'XYZ' AS c_varchar,
 
@@ -241,7 +241,7 @@ std::string ODBCMockTestBase::GetConnectionString() {
   std::string connect_str(
       "driver={Apache Arrow Flight SQL ODBC Driver};HOST=localhost;port=" +
       std::to_string(port) + ";token=" + std::string(kTestToken) +
-      ";useEncryption=false;");
+      ";useEncryption=false;UseWideChar=true;");
   return connect_str;
 }
 

cpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.h

@@ -237,6 +237,9 @@ 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 kErrorStateS1004 = "S1004";
+static constexpr std::string_view kErrorStateS1002 = "S1002";
+static constexpr std::string_view kErrorStateS1010 = "S1010";
+static constexpr std::string_view kErrorStateS1090 = "S1090";
 
 /// Verify ODBC Error State
 void VerifyOdbcErrorState(SQLSMALLINT handle_type, SQLHANDLE handle,