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
18 changes: 12 additions & 6 deletions cpp/src/arrow/flight/sql/odbc/entry_points.cc
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,18 @@ SQLRETURN SQL_API SQLGetDiagRec(SQLSMALLINT handle_type, SQLHANDLE handle,
buffer_length, text_length_ptr);
}

#if defined(__APPLE__)
// macOS ODBC Driver Manager doesn't map SQLError to SQLGetDiagRec, so we need to
// implement SQLError for macOS.
// on Windows, SQLError mapping implemented by Driver Manager is preferred.
SQLRETURN SQL_API SQLError(SQLHENV env, SQLHDBC conn, SQLHSTMT stmt, SQLWCHAR* sql_state,
SQLINTEGER* native_error_ptr, SQLWCHAR* message_text,
SQLSMALLINT buffer_length, SQLSMALLINT* text_length_ptr) {
return arrow::flight::sql::odbc::SQLError(env, conn, stmt, sql_state, native_error_ptr,
message_text, buffer_length, text_length_ptr);
}
#endif // __APPLE__

SQLRETURN SQL_API SQLGetEnvAttr(SQLHENV env, SQLINTEGER attr, SQLPOINTER value_ptr,
SQLINTEGER buffer_len, SQLINTEGER* str_len_ptr) {
return arrow::flight::sql::odbc::SQLGetEnvAttr(env, attr, value_ptr, buffer_len,
Expand Down Expand Up @@ -317,9 +329,3 @@ 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;
}
51 changes: 50 additions & 1 deletion cpp/src/arrow/flight/sql/odbc/odbc_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,56 @@ SQLRETURN SQLFreeStmt(SQLHSTMT handle, SQLUSMALLINT option) {
return SQL_ERROR;
}

#if defined(__APPLE__)

Choose a reason for hiding this comment

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

Do we also need a windows version of SQLError?

Copy link
Author

Choose a reason for hiding this comment

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

We can rely on the driver manager for Windows to provide an implementation of SQLError, it is less error prone.

Driver Managers are supposed to handle ODBC 2.0 APIs like SQLError by mapping to ODBC 3.0 APIs, but macOS Driver Manager is missing on the functionality so we have to implement SQLError for macOS.

SQLRETURN SQLError(SQLHENV env, SQLHDBC conn, SQLHSTMT stmt, SQLWCHAR* sql_state,
SQLINTEGER* native_error_ptr, SQLWCHAR* message_text,
SQLSMALLINT buffer_length, SQLSMALLINT* text_length_ptr) {
ARROW_LOG(DEBUG) << "SQLError called with env: " << env << ", conn: " << conn
<< ", stmt: " << stmt
<< ", sql_state: " << static_cast<const void*>(sql_state)
<< ", native_error_ptr: " << static_cast<const void*>(native_error_ptr)
<< ", message_text: " << static_cast<const void*>(message_text)
<< ", buffer_length: " << buffer_length
<< ", text_length_ptr: " << static_cast<const void*>(text_length_ptr);

SQLSMALLINT handle_type;
SQLHANDLE handle;

if (env) {
handle_type = SQL_HANDLE_ENV;
handle = static_cast<SQLHANDLE>(env);
} else if (conn) {
handle_type = SQL_HANDLE_DBC;
handle = static_cast<SQLHANDLE>(conn);
} else if (stmt) {
handle_type = SQL_HANDLE_STMT;
handle = static_cast<SQLHANDLE>(stmt);
} else {
return static_cast<SQLRETURN>(SQL_INVALID_HANDLE);
}

// Use the last record
SQLINTEGER diag_number;
SQLSMALLINT diag_number_length;

SQLRETURN ret = arrow::flight::sql::odbc::SQLGetDiagField(
handle_type, handle, 0, SQL_DIAG_NUMBER, &diag_number, sizeof(SQLINTEGER), 0);
if (ret != SQL_SUCCESS) {
return ret;
}

if (diag_number == 0) {
return SQL_NO_DATA;
}

SQLSMALLINT rec_number = static_cast<SQLSMALLINT>(diag_number);

return arrow::flight::sql::odbc::SQLGetDiagRec(
handle_type, handle, rec_number, sql_state, native_error_ptr, message_text,
buffer_length, text_length_ptr);
}
#endif // __APPLE__

inline bool IsValidStringFieldArgs(SQLPOINTER diag_info_ptr, SQLSMALLINT buffer_length,
SQLSMALLINT* string_length_ptr, bool is_unicode) {
const SQLSMALLINT char_size = is_unicode ? GetSqlWCharSize() : sizeof(char);
Expand Down Expand Up @@ -735,7 +785,6 @@ SQLRETURN SQLGetConnectAttr(SQLHDBC conn, SQLINTEGER attribute, SQLPOINTER value
<< ", attribute: " << attribute << ", value_ptr: " << value_ptr
<< ", buffer_length: " << buffer_length << ", string_length_ptr: "
<< static_cast<const void*>(string_length_ptr);

using ODBC::ODBCConnection;

return ODBCConnection::ExecuteWithDiagnostics(conn, SQL_ERROR, [=]() {
Expand Down
6 changes: 6 additions & 0 deletions cpp/src/arrow/flight/sql/odbc/odbc_api_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@ namespace arrow::flight::sql::odbc {
SQLHANDLE* result);
[[nodiscard]] SQLRETURN SQLFreeHandle(SQLSMALLINT type, SQLHANDLE handle);
[[nodiscard]] SQLRETURN SQLFreeStmt(SQLHSTMT stmt, SQLUSMALLINT option);
#if defined(__APPLE__)
[[nodiscard]] SQLRETURN SQLError(SQLHENV env, SQLHDBC conn, SQLHSTMT stmt,
SQLWCHAR* sql_state, SQLINTEGER* native_error_ptr,
SQLWCHAR* message_text, SQLSMALLINT buffer_length,
SQLSMALLINT* text_length_ptr);
#endif // __APPLE__
[[nodiscard]] SQLRETURN SQLGetDiagField(SQLSMALLINT handle_type, SQLHANDLE handle,
SQLSMALLINT rec_number,
SQLSMALLINT diag_identifier,
Expand Down
44 changes: 30 additions & 14 deletions cpp/src/arrow/flight/sql/odbc/tests/errors_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ using TestTypesHandle = ::testing::Types<FlightSQLOdbcEnvConnHandleMockTestBase,
FlightSQLOdbcEnvConnHandleRemoteTestBase>;
TYPED_TEST_SUITE(ErrorsHandleTest, TestTypesHandle);

using ODBC::SqlWcharToString;

TYPED_TEST(ErrorsHandleTest, TestSQLGetDiagFieldWForConnectFailure) {
// Invalid connect string
std::string connect_str = this->GetInvalidConnectionString();
Expand Down Expand Up @@ -119,7 +121,7 @@ TYPED_TEST(ErrorsHandleTest, TestSQLGetDiagFieldWForConnectFailure) {
SQLGetDiagField(SQL_HANDLE_DBC, this->conn, RECORD_1, SQL_DIAG_SQLSTATE, sql_state,
sql_state_size * GetSqlWCharSize(), &sql_state_length));

EXPECT_EQ(std::wstring(L"28000"), std::wstring(sql_state));
EXPECT_EQ(kErrorState28000, SqlWcharToString(sql_state));
}

TYPED_TEST(ErrorsHandleTest, DISABLED_TestSQLGetDiagFieldWForConnectFailureNTS) {
Expand Down Expand Up @@ -218,7 +220,7 @@ TYPED_TEST(ErrorsTest, TestSQLGetDiagFieldWForDescriptorFailureFromDriverManager
SQLGetDiagField(SQL_HANDLE_DESC, descriptor, RECORD_1, SQL_DIAG_SQLSTATE, sql_state,
sql_state_size * GetSqlWCharSize(), &sql_state_length));

EXPECT_EQ(std::wstring(L"IM001"), std::wstring(sql_state));
EXPECT_EQ(kErrorStateIM001, SqlWcharToString(sql_state));

// Free descriptor handle
EXPECT_EQ(SQL_SUCCESS, SQLFreeHandle(SQL_HANDLE_DESC, descriptor));
Expand Down Expand Up @@ -247,7 +249,7 @@ TYPED_TEST(ErrorsTest, TestSQLGetDiagRecForDescriptorFailureFromDriverManager) {
EXPECT_EQ(0, native_error);

// API not implemented error from driver manager
EXPECT_EQ(std::wstring(L"IM001"), std::wstring(sql_state));
EXPECT_EQ(kErrorStateIM001, SqlWcharToString(sql_state));

EXPECT_FALSE(std::wstring(message).empty());

Expand Down Expand Up @@ -284,7 +286,7 @@ TYPED_TEST(ErrorsHandleTest, TestSQLGetDiagRecForConnectFailure) {

EXPECT_EQ(200, native_error);

EXPECT_EQ(std::wstring(L"28000"), std::wstring(sql_state));
EXPECT_EQ(kErrorState28000, SqlWcharToString(sql_state));

EXPECT_FALSE(std::wstring(message).empty());
}
Expand Down Expand Up @@ -358,12 +360,12 @@ TYPED_TEST(ErrorsTest, TestSQLErrorEnvErrorFromDriverManager) {
ASSERT_EQ(SQL_SUCCESS, SQLError(this->env, nullptr, nullptr, sql_state, &native_error,
message, SQL_MAX_MESSAGE_LENGTH, &message_length));

EXPECT_GT(message_length, 50);
EXPECT_GT(message_length, 40);

EXPECT_EQ(0, native_error);

// Function sequence error state from driver manager
EXPECT_EQ(std::wstring(L"HY010"), std::wstring(sql_state));
EXPECT_EQ(kErrorStateHY010, SqlWcharToString(sql_state));

EXPECT_FALSE(std::wstring(message).empty());
}
Expand All @@ -390,7 +392,7 @@ TYPED_TEST(ErrorsTest, TestSQLErrorConnError) {
EXPECT_EQ(100, native_error);

// optional feature not supported error state
EXPECT_EQ(std::wstring(L"HYC00"), std::wstring(sql_state));
EXPECT_EQ(kErrorStateHYC00, SqlWcharToString(sql_state));

EXPECT_FALSE(std::wstring(message).empty());
}
Expand Down Expand Up @@ -420,7 +422,7 @@ TYPED_TEST(ErrorsTest, TestSQLErrorStmtError) {

EXPECT_EQ(100, native_error);

EXPECT_EQ(std::wstring(L"HY000"), std::wstring(sql_state));
EXPECT_EQ(kErrorStateHY000, SqlWcharToString(sql_state));

EXPECT_FALSE(std::wstring(message).empty());
}
Expand Down Expand Up @@ -457,7 +459,7 @@ TYPED_TEST(ErrorsTest, TestSQLErrorStmtWarning) {
EXPECT_EQ(1000100, native_error);

// Verify string truncation warning is reported
EXPECT_EQ(std::wstring(L"01004"), std::wstring(sql_state));
EXPECT_EQ(kErrorState01004, SqlWcharToString(sql_state));

EXPECT_FALSE(std::wstring(message).empty());
}
Expand All @@ -479,22 +481,35 @@ TYPED_TEST(ErrorsOdbcV2Test, TestSQLErrorEnvErrorFromDriverManager) {
ASSERT_EQ(SQL_SUCCESS, SQLError(this->env, nullptr, nullptr, sql_state, &native_error,
message, SQL_MAX_MESSAGE_LENGTH, &message_length));

EXPECT_GT(message_length, 50);
EXPECT_GT(message_length, 40);

EXPECT_EQ(0, native_error);

// Function sequence error state from driver manager
EXPECT_EQ(std::wstring(L"S1010"), std::wstring(sql_state));
#ifdef _WIN32
// Windows Driver Manager returns S1010
EXPECT_EQ(kErrorStateS1010, SqlWcharToString(sql_state));
#else
// unix Driver Manager returns HY010
EXPECT_EQ(kErrorStateHY010, SqlWcharToString(sql_state));
#endif // _WIN32

EXPECT_FALSE(std::wstring(message).empty());
}

// TODO: verify that `SQLGetConnectOption` is not required by Excel.
#ifndef __APPLE__
TYPED_TEST(ErrorsOdbcV2Test, TestSQLErrorConnError) {
Comment on lines +500 to 502
Copy link
Author

Choose a reason for hiding this comment

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

Still need to verify if SQLGetConnectOption is required by latest version of Excel. Will remove this todo comment in future PRs.

For this PR, we are checking in the SQLErrors changes.

// Test ODBC 2.0 API SQLError with ODBC ver 2.
// Known Windows Driver Manager (DM) behavior:
// When application passes buffer length greater than SQL_MAX_MESSAGE_LENGTH (512),
// DM passes 512 as buffer length to SQLError.

// Known macOS Driver Manager (DM) behavior:
// Attempts to call SQLGetConnectOption without redirecting the API call to
// SQLGetConnectAttr. SQLGetConnectOption is not implemented as it is not required by
// macOS Excel.

// Attempt to set unsupported attribute
ASSERT_EQ(SQL_ERROR,
SQLGetConnectAttr(this->conn, SQL_ATTR_TXN_ISOLATION, 0, 0, nullptr));
Expand All @@ -511,10 +526,11 @@ TYPED_TEST(ErrorsOdbcV2Test, TestSQLErrorConnError) {
EXPECT_EQ(100, native_error);

// optional feature not supported error state. Driver Manager maps state to S1C00
EXPECT_EQ(std::wstring(L"S1C00"), std::wstring(sql_state));
EXPECT_EQ(kErrorStateS1C00, SqlWcharToString(sql_state));

EXPECT_FALSE(std::wstring(message).empty());
}
#endif // __APPLE__

TYPED_TEST(ErrorsOdbcV2Test, TestSQLErrorStmtError) {
// Test ODBC 2.0 API SQLError with ODBC ver 2.
Expand All @@ -540,7 +556,7 @@ TYPED_TEST(ErrorsOdbcV2Test, TestSQLErrorStmtError) {
EXPECT_EQ(100, native_error);

// Driver Manager maps error state to S1000
EXPECT_EQ(std::wstring(L"S1000"), std::wstring(sql_state));
EXPECT_EQ(kErrorStateS1000, SqlWcharToString(sql_state));

EXPECT_FALSE(std::wstring(message).empty());
}
Expand Down Expand Up @@ -577,7 +593,7 @@ TYPED_TEST(ErrorsOdbcV2Test, TestSQLErrorStmtWarning) {
EXPECT_EQ(1000100, native_error);

// Verify string truncation warning is reported
EXPECT_EQ(std::wstring(L"01004"), std::wstring(sql_state));
EXPECT_EQ(kErrorState01004, SqlWcharToString(sql_state));

EXPECT_FALSE(std::wstring(message).empty());
}
Expand Down
3 changes: 3 additions & 0 deletions cpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.h
Original file line number Diff line number Diff line change
Expand Up @@ -236,10 +236,13 @@ static constexpr std::string_view kErrorStateHY106 = "HY106";
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 kErrorStateIM001 = "IM001";
static constexpr std::string_view kErrorStateS1000 = "S1000";
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";
static constexpr std::string_view kErrorStateS1C00 = "S1C00";

/// Verify ODBC Error State
void VerifyOdbcErrorState(SQLSMALLINT handle_type, SQLHANDLE handle,
Expand Down
Loading