diff --git a/cpp/src/arrow/flight/sql/odbc/entry_points.cc b/cpp/src/arrow/flight/sql/odbc/entry_points.cc index 9c571b582cc..c03089ffcc6 100644 --- a/cpp/src/arrow/flight/sql/odbc/entry_points.cc +++ b/cpp/src/arrow/flight/sql/odbc/entry_points.cc @@ -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, @@ -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; -} diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_api.cc b/cpp/src/arrow/flight/sql/odbc/odbc_api.cc index 1d5ec5a500e..2af1543b84c 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_api.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_api.cc @@ -247,6 +247,56 @@ SQLRETURN SQLFreeStmt(SQLHSTMT handle, SQLUSMALLINT option) { return SQL_ERROR; } +#if defined(__APPLE__) +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(sql_state) + << ", native_error_ptr: " << static_cast(native_error_ptr) + << ", message_text: " << static_cast(message_text) + << ", buffer_length: " << buffer_length + << ", text_length_ptr: " << static_cast(text_length_ptr); + + SQLSMALLINT handle_type; + SQLHANDLE handle; + + if (env) { + handle_type = SQL_HANDLE_ENV; + handle = static_cast(env); + } else if (conn) { + handle_type = SQL_HANDLE_DBC; + handle = static_cast(conn); + } else if (stmt) { + handle_type = SQL_HANDLE_STMT; + handle = static_cast(stmt); + } else { + return static_cast(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(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); @@ -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(string_length_ptr); - using ODBC::ODBCConnection; return ODBCConnection::ExecuteWithDiagnostics(conn, SQL_ERROR, [=]() { diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_api_internal.h b/cpp/src/arrow/flight/sql/odbc/odbc_api_internal.h index 4fea8569acb..f9d8d887cb8 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_api_internal.h +++ b/cpp/src/arrow/flight/sql/odbc/odbc_api_internal.h @@ -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, diff --git a/cpp/src/arrow/flight/sql/odbc/tests/errors_test.cc b/cpp/src/arrow/flight/sql/odbc/tests/errors_test.cc index 53ac33579a8..b38e10a6ce8 100644 --- a/cpp/src/arrow/flight/sql/odbc/tests/errors_test.cc +++ b/cpp/src/arrow/flight/sql/odbc/tests/errors_test.cc @@ -47,6 +47,8 @@ using TestTypesHandle = ::testing::Types; TYPED_TEST_SUITE(ErrorsHandleTest, TestTypesHandle); +using ODBC::SqlWcharToString; + TYPED_TEST(ErrorsHandleTest, TestSQLGetDiagFieldWForConnectFailure) { // Invalid connect string std::string connect_str = this->GetInvalidConnectionString(); @@ -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) { @@ -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)); @@ -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()); @@ -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()); } @@ -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()); } @@ -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()); } @@ -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()); } @@ -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()); } @@ -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) { // 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)); @@ -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. @@ -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()); } @@ -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()); } diff --git a/cpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.h b/cpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.h index aee423451c4..7d24216b9fd 100644 --- a/cpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.h +++ b/cpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.h @@ -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,