Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,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\install\install_amd64.cmd" ${{github.workspace}}\build\cpp\%ARROW_BUILD_TYPE%\libarrow_flight_sql_odbc.dll
- name: Test
shell: msys2 {0}
run: |
Expand Down
2 changes: 0 additions & 2 deletions ci/scripts/cpp_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,6 @@ case "$(uname)" in
exclude_tests="${exclude_tests}|gandiva-precompiled-test"
exclude_tests="${exclude_tests}|gandiva-projector-test"
exclude_tests="${exclude_tests}|gandiva-utf8-test"
# TODO: Enable ODBC tests
exclude_tests="${exclude_tests}|arrow-connection-test"
ctest_options+=(--exclude-regex "${exclude_tests}")
;;
*)
Expand Down
2 changes: 1 addition & 1 deletion cpp/cmake_modules/DefineOptions.cmake
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)
Copy link
Author

Choose a reason for hiding this comment

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

We want to enable builds on clang64 and MinGW too. The driver should work on any Win32 platform

set(ARROW_FLIGHT_SQL_ODBC OFF)
endif()
if(MSVC_TOOLCHAIN)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ class NoOpAuthMethod : public FlightSqlAuthMethod {
void Authenticate(FlightSqlConnection& connection,
FlightCallOptions& call_options) override {
// Do nothing

// TODO: implement NoOpAuthMethod to validate server address.
// Can use NoOpClientAuthHandler.
// https://github.com/apache/arrow/issues/46733
}
};

Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/flight/sql/odbc/flight_sql/system_dsn.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ bool DisplayConnectionWindow(void* windowParent, Configuration& config);
* @param windowParent Parent window handle.
* @param config Output configuration, presumed to be empty, it will be using values from
* properties.
* @param config Output properties.
* @param properties Output properties.
* @return True on success and false on fail.
*/
bool DisplayConnectionWindow(void* windowParent, Configuration& config,
Expand Down
12 changes: 12 additions & 0 deletions cpp/src/arrow/flight/sql/odbc/tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,25 @@ include_directories(${ODBC_INCLUDE_DIRS})

add_definitions(-DUNICODE=1)

find_package(SQLite3Alt REQUIRED)

set(ARROW_FLIGHT_SQL_MOCK_SERVER_SRCS
../../example/sqlite_sql_info.cc
../../example/sqlite_type_info.cc
../../example/sqlite_statement.cc
../../example/sqlite_statement_batch_reader.cc
../../example/sqlite_server.cc
../../example/sqlite_tables_schema_batch_reader.cc)

add_arrow_test(connection_test
SOURCES
connection_test.cc
odbc_test_suite.cc
odbc_test_suite.h
${ARROW_FLIGHT_SQL_MOCK_SERVER_SRCS}
EXTRA_LINK_LIBS
${ODBC_LIBRARIES}
${ODBCINST}
${SQLite3_LIBRARIES}
arrow_odbc_spi_impl
odbcabstraction)
111 changes: 51 additions & 60 deletions cpp/src/arrow/flight/sql/odbc/tests/connection_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -200,49 +200,51 @@ TEST(SQLSetEnvAttr, TestSQLSetEnvAttrODBCVersionInvalid) {
EXPECT_TRUE(return_set == SQL_ERROR);
}

TEST_F(FlightSQLODBCTestBase, TestSQLGetEnvAttrOutputNTS) {
connect();
TYPED_TEST(FlightSQLODBCTestBase, TestSQLGetEnvAttrOutputNTS) {
this->connect();
Comment on lines +203 to +204
Copy link
Author

Choose a reason for hiding this comment

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

We need to use this to indicate where we're getting the function when using TYPED_TEST


SQLINTEGER output_nts;

SQLRETURN return_get = SQLGetEnvAttr(env, SQL_ATTR_OUTPUT_NTS, &output_nts, 0, 0);
SQLRETURN return_get = SQLGetEnvAttr(this->env, SQL_ATTR_OUTPUT_NTS, &output_nts, 0, 0);

EXPECT_TRUE(return_get == SQL_SUCCESS);

EXPECT_EQ(output_nts, SQL_TRUE);

disconnect();
this->disconnect();
}

TEST_F(FlightSQLODBCTestBase, TestSQLGetEnvAttrGetLength) {
TYPED_TEST(FlightSQLODBCTestBase, TestSQLGetEnvAttrGetLength) {
// Test is disabled because call to SQLGetEnvAttr is handled by the driver manager on
// Windows. This test case can be potentionally used on macOS/Linux
// Windows. This test case can be potentially used on macOS/Linux
GTEST_SKIP();

connect();
this->connect();

SQLINTEGER length;

SQLRETURN return_get = SQLGetEnvAttr(env, SQL_ATTR_ODBC_VERSION, nullptr, 0, &length);
SQLRETURN return_get =
SQLGetEnvAttr(this->env, SQL_ATTR_ODBC_VERSION, nullptr, 0, &length);

EXPECT_TRUE(return_get == SQL_SUCCESS);

EXPECT_EQ(length, sizeof(SQLINTEGER));

disconnect();
this->disconnect();
}

TEST_F(FlightSQLODBCTestBase, TestSQLGetEnvAttrNullValuePointer) {
TYPED_TEST(FlightSQLODBCTestBase, TestSQLGetEnvAttrNullValuePointer) {
// Test is disabled because call to SQLGetEnvAttr is handled by the driver manager on
// Windows. This test case can be potentionally used on macOS/Linux
// Windows. This test case can be potentially used on macOS/Linux
GTEST_SKIP();
connect();
this->connect();

SQLRETURN return_get = SQLGetEnvAttr(env, SQL_ATTR_ODBC_VERSION, nullptr, 0, nullptr);
SQLRETURN return_get =
SQLGetEnvAttr(this->env, SQL_ATTR_ODBC_VERSION, nullptr, 0, nullptr);

EXPECT_TRUE(return_get == SQL_ERROR);

disconnect();
this->disconnect();
}

TEST(SQLSetEnvAttr, TestSQLSetEnvAttrOutputNTSValid) {
Expand Down Expand Up @@ -292,7 +294,7 @@ TEST(SQLSetEnvAttr, TestSQLSetEnvAttrNullValuePointer) {
EXPECT_TRUE(return_set == SQL_ERROR);
}

TEST(SQLDriverConnect, TestSQLDriverConnect) {
TYPED_TEST(FlightSQLODBCTestBase, TestSQLDriverConnect) {
// ODBC Environment
SQLHENV env;
SQLHDBC conn;
Expand All @@ -312,8 +314,7 @@ TEST(SQLDriverConnect, TestSQLDriverConnect) {
EXPECT_TRUE(ret == SQL_SUCCESS);

// Connect string
ASSERT_OK_AND_ASSIGN(std::string connect_str,
arrow::internal::GetEnvVar(TEST_CONNECT_STR));
std::string connect_str = this->getConnectionString();
ASSERT_OK_AND_ASSIGN(std::wstring wconnect_str,
arrow::util::UTF8ToWideString(connect_str));
std::vector<SQLWCHAR> connect_str0(wconnect_str.begin(), wconnect_str.end());
Expand Down Expand Up @@ -361,7 +362,7 @@ TEST(SQLDriverConnect, TestSQLDriverConnect) {
EXPECT_TRUE(ret == SQL_SUCCESS);
}

TEST(SQLDriverConnect, TestSQLDriverConnectInvalidUid) {
TEST_F(FlightSQLODBCRemoteTestBase, TestSQLDriverConnectInvalidUid) {
// ODBC Environment
SQLHENV env;
SQLHDBC conn;
Expand All @@ -380,11 +381,8 @@ TEST(SQLDriverConnect, TestSQLDriverConnectInvalidUid) {

EXPECT_TRUE(ret == SQL_SUCCESS);

// Connect string
ASSERT_OK_AND_ASSIGN(std::string connect_str,
arrow::internal::GetEnvVar(TEST_CONNECT_STR));
// Append invalid uid to connection string
connect_str += std::string("uid=non_existent_id;");
// Invalid connect string
std::string connect_str = getInvalidConnectionString();

ASSERT_OK_AND_ASSIGN(std::wstring wconnect_str,
arrow::util::UTF8ToWideString(connect_str));
Expand Down Expand Up @@ -418,7 +416,7 @@ TEST(SQLDriverConnect, TestSQLDriverConnectInvalidUid) {
EXPECT_TRUE(ret == SQL_SUCCESS);
}

TEST(SQLConnect, TestSQLConnect) {
TYPED_TEST(FlightSQLODBCTestBase, TestSQLConnect) {
// ODBC Environment
SQLHENV env;
SQLHDBC conn;
Expand All @@ -438,8 +436,7 @@ TEST(SQLConnect, TestSQLConnect) {
EXPECT_TRUE(ret == SQL_SUCCESS);

// Connect string
ASSERT_OK_AND_ASSIGN(std::string connect_str,
arrow::internal::GetEnvVar(TEST_CONNECT_STR));
std::string connect_str = this->getConnectionString();

// Write connection string content into a DSN,
// must succeed before continuing
Expand All @@ -454,7 +451,7 @@ TEST(SQLConnect, TestSQLConnect) {
std::vector<SQLWCHAR> uid0(wuid.begin(), wuid.end());
std::vector<SQLWCHAR> pwd0(wpwd.begin(), wpwd.end());

// Connecting to ODBC server.
// Connecting to ODBC server. Empty uid and pwd should be ignored.
ret = SQLConnect(conn, dsn0.data(), static_cast<SQLSMALLINT>(dsn0.size()), uid0.data(),
static_cast<SQLSMALLINT>(uid0.size()), pwd0.data(),
static_cast<SQLSMALLINT>(pwd0.size()));
Expand Down Expand Up @@ -488,7 +485,7 @@ TEST(SQLConnect, TestSQLConnect) {
EXPECT_TRUE(ret == SQL_SUCCESS);
}

TEST(SQLConnect, TestSQLConnectInputUidPwd) {
TEST_F(FlightSQLODBCRemoteTestBase, TestSQLConnectInputUidPwd) {
// ODBC Environment
SQLHENV env;
SQLHDBC conn;
Expand All @@ -508,10 +505,9 @@ TEST(SQLConnect, TestSQLConnectInputUidPwd) {
EXPECT_TRUE(ret == SQL_SUCCESS);

// Connect string
ASSERT_OK_AND_ASSIGN(std::string connect_str,
arrow::internal::GetEnvVar(TEST_CONNECT_STR));
std::string connect_str = getConnectionString();

// Retrieve valid uid and pwd
// Retrieve valid uid and pwd, assumes TEST_CONNECT_STR contains uid and pwd
Connection::ConnPropertyMap properties;
ODBC::ODBCConnection::getPropertiesFromConnString(connect_str, properties);
std::string uid_key("uid");
Expand Down Expand Up @@ -567,7 +563,7 @@ TEST(SQLConnect, TestSQLConnectInputUidPwd) {
EXPECT_TRUE(ret == SQL_SUCCESS);
}

TEST(SQLConnect, TestSQLConnectInvalidUid) {
TEST_F(FlightSQLODBCRemoteTestBase, TestSQLConnectInvalidUid) {
// ODBC Environment
SQLHENV env;
SQLHDBC conn;
Expand All @@ -587,10 +583,9 @@ TEST(SQLConnect, TestSQLConnectInvalidUid) {
EXPECT_TRUE(ret == SQL_SUCCESS);

// Connect string
ASSERT_OK_AND_ASSIGN(std::string connect_str,
arrow::internal::GetEnvVar(TEST_CONNECT_STR));
std::string connect_str = getConnectionString();

// Retrieve valid uid and pwd
// Retrieve valid uid and pwd, assumes TEST_CONNECT_STR contains uid and pwd
Connection::ConnPropertyMap properties;
ODBC::ODBCConnection::getPropertiesFromConnString(connect_str, properties);
std::string uid = properties[std::string("uid")];
Expand Down Expand Up @@ -636,7 +631,7 @@ TEST(SQLConnect, TestSQLConnectInvalidUid) {
EXPECT_TRUE(ret == SQL_SUCCESS);
}

TEST(SQLConnect, TestSQLConnectDSNPrecedence) {
TEST_F(FlightSQLODBCRemoteTestBase, TestSQLConnectDSNPrecedence) {
// ODBC Environment
SQLHENV env;
SQLHDBC conn;
Expand All @@ -656,13 +651,13 @@ TEST(SQLConnect, TestSQLConnectDSNPrecedence) {
EXPECT_TRUE(ret == SQL_SUCCESS);

// Connect string
ASSERT_OK_AND_ASSIGN(std::string connect_str,
arrow::internal::GetEnvVar(TEST_CONNECT_STR));
std::string connect_str = getConnectionString();

// Write connection string content into a DSN,
// must succeed before continuing

// Pass incorrect uid and password to SQLConnect, they will be ignored
// Pass incorrect uid and password to SQLConnect, they will be ignored.
// Assumes TEST_CONNECT_STR contains uid and pwd
std::string uid("non_existent_id"), pwd("non_existent_password");
ASSERT_TRUE(writeDSN(connect_str));

Expand Down Expand Up @@ -746,7 +741,7 @@ TEST(SQLDisconnect, TestSQLDisconnectWithoutConnection) {
EXPECT_TRUE(ret == SQL_SUCCESS);
}

TEST(SQLGetDiagFieldW, TestSQLGetDiagFieldWForConnectFailure) {
TYPED_TEST(FlightSQLODBCTestBase, TestSQLGetDiagFieldWForConnectFailure) {
// ODBC Environment
SQLHENV env;
SQLHDBC conn;
Expand All @@ -765,11 +760,8 @@ TEST(SQLGetDiagFieldW, TestSQLGetDiagFieldWForConnectFailure) {

EXPECT_TRUE(ret == SQL_SUCCESS);

// Connect string
ASSERT_OK_AND_ASSIGN(std::string connect_str,
arrow::internal::GetEnvVar(TEST_CONNECT_STR));
// Append invalid uid to connection string
connect_str += std::string("uid=non_existent_id;");
// Invalid connect string
std::string connect_str = this->getInvalidConnectionString();

ASSERT_OK_AND_ASSIGN(std::wstring wconnect_str,
arrow::util::UTF8ToWideString(connect_str));
Expand Down Expand Up @@ -859,9 +851,9 @@ TEST(SQLGetDiagFieldW, TestSQLGetDiagFieldWForConnectFailure) {
EXPECT_TRUE(ret == SQL_SUCCESS);
}

TEST(SQLGetDiagFieldW, TestSQLGetDiagFieldWForConnectFailureNTS) {
TYPED_TEST(FlightSQLODBCTestBase, TestSQLGetDiagFieldWForConnectFailureNTS) {
// Test is disabled because driver manager on Windows does not pass through SQL_NTS
// This test case can be potentionally used on macOS/Linux
// This test case can be potentially used on macOS/Linux
GTEST_SKIP();
// ODBC Environment
SQLHENV env;
Expand All @@ -881,11 +873,8 @@ TEST(SQLGetDiagFieldW, TestSQLGetDiagFieldWForConnectFailureNTS) {

EXPECT_TRUE(ret == SQL_SUCCESS);

// Connect string
ASSERT_OK_AND_ASSIGN(std::string connect_str,
arrow::internal::GetEnvVar(TEST_CONNECT_STR));
// Append invalid uid to connection string
connect_str += std::string("uid=non_existent_id;");
// Invalid connect string
std::string connect_str = this->getInvalidConnectionString();

ASSERT_OK_AND_ASSIGN(std::wstring wconnect_str,
arrow::util::UTF8ToWideString(connect_str));
Expand All @@ -902,7 +891,6 @@ TEST(SQLGetDiagFieldW, TestSQLGetDiagFieldWForConnectFailureNTS) {
EXPECT_TRUE(ret == SQL_ERROR);

// Retrieve all supported header level and record level data
SQLSMALLINT HEADER_LEVEL = 0;
SQLSMALLINT RECORD_1 = 1;

// SQL_DIAG_MESSAGE_TEXT SQL_NTS
Expand All @@ -929,7 +917,7 @@ TEST(SQLGetDiagFieldW, TestSQLGetDiagFieldWForConnectFailureNTS) {
EXPECT_TRUE(ret == SQL_SUCCESS);
}

TEST(SQLGetDiagRec, TestSQLGetDiagRecForConnectFailure) {
TYPED_TEST(FlightSQLODBCTestBase, TestSQLGetDiagRecForConnectFailure) {
// ODBC Environment
SQLHENV env;
SQLHDBC conn;
Expand All @@ -948,11 +936,8 @@ TEST(SQLGetDiagRec, TestSQLGetDiagRecForConnectFailure) {

EXPECT_TRUE(ret == SQL_SUCCESS);

// Connect string
ASSERT_OK_AND_ASSIGN(std::string connect_str,
arrow::internal::GetEnvVar(TEST_CONNECT_STR));
// Append invalid uid to connection string
connect_str += std::string("uid=non_existent_id;");
// Invalid connect string
std::string connect_str = this->getInvalidConnectionString();

ASSERT_OK_AND_ASSIGN(std::wstring wconnect_str,
arrow::util::UTF8ToWideString(connect_str));
Expand All @@ -978,7 +963,7 @@ TEST(SQLGetDiagRec, TestSQLGetDiagRecForConnectFailure) {

EXPECT_TRUE(ret == SQL_SUCCESS);

EXPECT_GT(message_length, 200);
Comment on lines 965 to -981
Copy link
Author

Choose a reason for hiding this comment

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

The error message from mock server is only 129 characters, so I adjusted this check

EXPECT_GT(message_length, 120);

EXPECT_EQ(native_error, 200);

Expand All @@ -1000,6 +985,12 @@ TEST(SQLGetDiagRec, TestSQLGetDiagRecForConnectFailure) {
EXPECT_TRUE(ret == SQL_SUCCESS);
}

TYPED_TEST(FlightSQLODBCTestBase, TestConnect) {
// Verifies connect and disconnect works on its own
this->connect();
this->disconnect();
}

} // namespace integration_tests
} // namespace odbc
} // namespace flight
Expand Down
Loading
Loading