Skip to content

GH-49268: [C++][FlightRPC] Fix ODBC tests for MacOS #49267

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
justing-bq wants to merge 3 commits into
base: main
Choose a base branch
from
Open

GH-49268: [C++][FlightRPC] Fix ODBC tests for MacOS #49267

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e091f57
Fix Mac Tests
alinaliBQ Mar 6, 2026
795a1d0
Resolve Unix-odbc vs. IODBC linking issue
alinaliBQ Mar 6, 2026
5c6228c
Change `GetInfo` disabled tests to start new connection
alinaliBQ Mar 10, 2026
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
3 changes: 3 additions & 0 deletions .github/workflows/cpp_extra.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,9 @@ jobs:
sudo sysctl -w kern.coredump=1
sudo sysctl -w kern.corefile=/tmp/core.%N.%P
ulimit -c unlimited # must enable within the same shell
# Give CI write access for system DSN
sudo mkdir -p /Library/ODBC
sudo chown -R $USER /Library/ODBC
ci/scripts/cpp_test.sh $(pwd) $(pwd)/build

odbc-msvc:
Expand Down
1 change: 0 additions & 1 deletion ci/scripts/cpp_test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ case "$(uname)" in
;;
Darwin)
n_jobs=$(sysctl -n hw.ncpu)
exclude_tests+=("arrow-flight-sql-odbc-test")
# TODO: https://github.com/apache/arrow/issues/40410
exclude_tests+=("arrow-s3fs-test")
;;
Expand Down
54 changes: 32 additions & 22 deletions cpp/src/arrow/flight/sql/example/sqlite_tables_schema_batch_reader.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,33 +65,43 @@ Status SqliteTablesWithSchemaBatchReader::ReadNext(std::shared_ptr<RecordBatch>*

auto* string_array = reinterpret_cast<StringArray*>(table_name_array.get());

std::vector<std::shared_ptr<Field>> column_fields;
std::map<std::string, std::vector<std::shared_ptr<Field>>> table_columns_map;
for (int i = 0; i < table_name_array->length(); i++) {
const std::string& table_name = string_array->GetString(i);
table_columns_map[table_name];
}

while (sqlite3_step(schema_statement->GetSqlite3Stmt()) == SQLITE_ROW) {
std::string sqlite_table_name = std::string(reinterpret_cast<const char*>(
sqlite3_column_text(schema_statement->GetSqlite3Stmt(), 0)));
if (sqlite_table_name == table_name) {
const char* column_name = reinterpret_cast<const char*>(
sqlite3_column_text(schema_statement->GetSqlite3Stmt(), 1));
const char* column_type = reinterpret_cast<const char*>(
sqlite3_column_text(schema_statement->GetSqlite3Stmt(), 2));
int nullable = sqlite3_column_int(schema_statement->GetSqlite3Stmt(), 3);

const ColumnMetadata& column_metadata = GetColumnMetadata(
GetSqlTypeFromTypeName(column_type), sqlite_table_name.c_str());
std::shared_ptr<DataType> arrow_type;
auto status = GetArrowType(column_type).Value(&arrow_type);
if (!status.ok()) {
return Status::NotImplemented("Unknown SQLite type '", column_type,
"' for column '", column_name, "' in table '",
table_name, "': ", status);
}
column_fields.push_back(arrow::field(column_name, arrow_type, nullable == 0,
column_metadata.metadata_map()));
while (sqlite3_step(schema_statement->GetSqlite3Stmt()) == SQLITE_ROW) {
std::string table_name = std::string(reinterpret_cast<const char*>(
sqlite3_column_text(schema_statement->GetSqlite3Stmt(), 0)));

if (table_columns_map.contains(table_name)) {
const char* column_name = reinterpret_cast<const char*>(
sqlite3_column_text(schema_statement->GetSqlite3Stmt(), 1));
const char* column_type = reinterpret_cast<const char*>(
sqlite3_column_text(schema_statement->GetSqlite3Stmt(), 2));
int nullable = sqlite3_column_int(schema_statement->GetSqlite3Stmt(), 3);

const ColumnMetadata& column_metadata =
GetColumnMetadata(GetSqlTypeFromTypeName(column_type), table_name.c_str());

std::shared_ptr<DataType> arrow_type;
auto status = GetArrowType(column_type).Value(&arrow_type);
if (!status.ok()) {
return Status::NotImplemented("Unknown SQLite type '", column_type,
"' for column '", column_name, "' in table '",
table_name, "': ", status);
}
table_columns_map[table_name].push_back(arrow::field(
column_name, arrow_type, nullable == 0, column_metadata.metadata_map()));
}
}

std::vector<std::shared_ptr<Field>> column_fields;
for (int i = 0; i < table_name_array->length(); i++) {
const std::string& table_name = string_array->GetString(i);
column_fields = table_columns_map[table_name];

ARROW_ASSIGN_OR_RAISE(std::shared_ptr<Buffer> schema_buffer,
ipc::SerializeSchema(*arrow::schema(column_fields)));

Expand Down
6 changes: 3 additions & 3 deletions cpp/src/arrow/flight/sql/odbc/odbc_impl/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ add_library(arrow_odbc_spi_impl STATIC
spi/result_set.h
spi/result_set_metadata.h
spi/statement.h
system_dsn.cc
system_dsn.h
system_trust_store.cc
system_trust_store.h
types.h
Expand All @@ -125,9 +127,7 @@ if(WIN32)
ui/dsn_configuration_window.h
ui/window.cc
ui/window.h
win_system_dsn.cc
system_dsn.cc
system_dsn.h)
win_system_dsn.cc)
endif()

if(APPLE)
Expand Down
4 changes: 2 additions & 2 deletions cpp/src/arrow/flight/sql/odbc/odbc_impl/odbc_statement.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,7 @@ void ODBCStatement::SetStmtAttr(SQLINTEGER statement_attribute, SQLPOINTER value
switch (statement_attribute) {
case SQL_ATTR_APP_PARAM_DESC: {
ODBCDescriptor* desc = static_cast<ODBCDescriptor*>(value);
if (current_apd_ != desc) {
if (desc && current_apd_ != desc) {
if (current_apd_ != built_in_apd_.get()) {
current_apd_->DetachFromStatement(this, true);
}
Expand All @@ -567,7 +567,7 @@ void ODBCStatement::SetStmtAttr(SQLINTEGER statement_attribute, SQLPOINTER value
}
case SQL_ATTR_APP_ROW_DESC: {
ODBCDescriptor* desc = static_cast<ODBCDescriptor*>(value);
if (current_ard_ != desc) {
if (desc && current_ard_ != desc) {
if (current_ard_ != built_in_ard_.get()) {
current_ard_->DetachFromStatement(this, false);
}
Expand Down
14 changes: 5 additions & 9 deletions cpp/src/arrow/flight/sql/odbc/odbc_impl/system_dsn.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,23 +17,19 @@

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

#include "arrow/flight/sql/odbc/odbc_impl/config/configuration.h"
#include "arrow/flight/sql/odbc/odbc_impl/flight_sql_connection.h"
#include "arrow/flight/sql/odbc/odbc_impl/ui/dsn_configuration_window.h"
#include "arrow/flight/sql/odbc/odbc_impl/ui/window.h"
#include "arrow/flight/sql/odbc/odbc_impl/util.h"
#include "arrow/result.h"
#include "arrow/util/utf8.h"

#include <odbcinst.h>
#include <sstream>

namespace arrow::flight::sql::odbc {

using config::Configuration;

void PostError(DWORD error_code, LPCWSTR error_msg) {
void PostError(DWORD error_code, LPWSTR error_msg) {
#if defined _WIN32
MessageBox(NULL, error_msg, L"Error!", MB_ICONEXCLAMATION | MB_OK);
#endif // _WIN32
SQLPostInstallerError(error_code, error_msg);
}

Expand All @@ -42,7 +38,7 @@ 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, werror_msg.c_str());
PostError(ODBC_ERROR_GENERAL_ERR, (LPWSTR)werror_msg.c_str());
}

void PostLastInstallerError() {
Expand All @@ -55,7 +51,7 @@ void PostLastInstallerError() {
buf << L"Message: \"" << msg << L"\", Code: " << code;
std::wstring error_msg = buf.str();

PostError(code, error_msg.c_str());
PostError(code, (LPWSTR)error_msg.c_str());
}

/**
Expand Down
5 changes: 4 additions & 1 deletion cpp/src/arrow/flight/sql/odbc/odbc_impl/system_dsn.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,11 @@
#include "arrow/flight/sql/odbc/odbc_impl/platform.h"

#include "arrow/flight/sql/odbc/odbc_impl/config/configuration.h"
#include "arrow/flight/sql/odbc/odbc_impl/flight_sql_connection.h"
#include "arrow/status.h"

#include <odbcinst.h>

namespace arrow::flight::sql::odbc {

#if defined _WIN32
Expand Down Expand Up @@ -64,7 +67,7 @@ bool RegisterDsn(const config::Configuration& config, LPCWSTR driver);
*/
bool UnregisterDsn(const std::wstring& dsn);

void PostError(DWORD error_code, LPCWSTR error_msg);
void PostError(DWORD error_code, LPWSTR error_msg);

void PostArrowUtilError(arrow::Status error_status);
} // namespace arrow::flight::sql::odbc
2 changes: 1 addition & 1 deletion cpp/src/arrow/flight/sql/odbc/odbc_impl/win_system_dsn.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ BOOL INSTAPI ConfigDSNW(HWND hwnd_parent, WORD req, LPCWSTR wdriver,
std::wstring werror_msg =
arrow::util::UTF8ToWideString(error_msg).ValueOr(L"Error during DSN load");

PostError(err.GetNativeError(), werror_msg.c_str());
PostError(err.GetNativeError(), (LPWSTR)werror_msg.c_str());
return FALSE;
}

Expand Down
3 changes: 2 additions & 1 deletion cpp/src/arrow/flight/sql/odbc/tests/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ else()
${ARROW_TEST_SHARED_LINK_LIBS})
endif()

set(ARROW_FLIGHT_SQL_ODBC_TEST_LIBS ${ODBC_LIBRARIES} ${ODBCINST} ${SQLite3_LIBRARIES})
# On macOS, link `ODBCINST` first to ensure iodbc take precedence over unixodbc
set(ARROW_FLIGHT_SQL_ODBC_TEST_LIBS ${ODBCINST} ${ODBC_LIBRARIES} ${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.
Expand Down
Loading
Loading