From c4de836ad25142d8daaca0380f5aba0beec7b358 Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Wed, 22 Oct 2025 11:53:39 -0700 Subject: [PATCH] Extract implementation for SQLGetEnvAttr and SQLSetEnvAttr Co-authored-by: rscales Add tests for setting and getting environment attributes Address code review comments Co-Authored-By: justing-bq --- cpp/src/arrow/flight/sql/odbc/odbc_api.cc | 97 ++++++++++++++- .../flight/sql/odbc/tests/connection_test.cc | 114 ++++++++++++++++++ 2 files changed, 207 insertions(+), 4 deletions(-) diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_api.cc b/cpp/src/arrow/flight/sql/odbc/odbc_api.cc index bce7c10e82b..ade3ecbe2b6 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_api.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_api.cc @@ -253,16 +253,105 @@ SQLRETURN SQLGetEnvAttr(SQLHENV env, SQLINTEGER attr, SQLPOINTER value_ptr, ARROW_LOG(DEBUG) << "SQLGetEnvAttr called with env: " << env << ", attr: " << attr << ", value_ptr: " << value_ptr << ", buffer_length: " << buffer_length << ", str_len_ptr: " << static_cast(str_len_ptr); - // GH-46575 TODO: Implement SQLGetEnvAttr - return SQL_INVALID_HANDLE; + + using ODBC::ODBCEnvironment; + + ODBCEnvironment* environment = reinterpret_cast(env); + + return ODBCEnvironment::ExecuteWithDiagnostics(environment, SQL_ERROR, [=]() { + switch (attr) { + case SQL_ATTR_ODBC_VERSION: { + if (!value_ptr && !str_len_ptr) { + throw DriverException("Invalid null pointer for attribute.", "HY000"); + } + + if (value_ptr) { + SQLINTEGER* value = reinterpret_cast(value_ptr); + *value = static_cast(environment->GetODBCVersion()); + } + + if (str_len_ptr) { + *str_len_ptr = sizeof(SQLINTEGER); + } + + return SQL_SUCCESS; + } + + case SQL_ATTR_OUTPUT_NTS: { + if (!value_ptr && !str_len_ptr) { + throw DriverException("Invalid null pointer for attribute.", "HY000"); + } + + if (value_ptr) { + // output nts always returns SQL_TRUE + SQLINTEGER* value = reinterpret_cast(value_ptr); + *value = SQL_TRUE; + } + + if (str_len_ptr) { + *str_len_ptr = sizeof(SQLINTEGER); + } + + return SQL_SUCCESS; + } + + case SQL_ATTR_CONNECTION_POOLING: { + throw DriverException("Optional feature not supported.", "HYC00"); + } + + default: { + throw DriverException("Invalid attribute", "HYC00"); + } + } + }); } SQLRETURN SQLSetEnvAttr(SQLHENV env, SQLINTEGER attr, SQLPOINTER value_ptr, SQLINTEGER str_len) { ARROW_LOG(DEBUG) << "SQLSetEnvAttr called with env: " << env << ", attr: " << attr << ", value_ptr: " << value_ptr << ", str_len: " << str_len; - // GH-46575 TODO: Implement SQLSetEnvAttr - return SQL_INVALID_HANDLE; + + using ODBC::ODBCEnvironment; + + ODBCEnvironment* environment = reinterpret_cast(env); + + return ODBCEnvironment::ExecuteWithDiagnostics(environment, SQL_ERROR, [=]() { + if (!value_ptr) { + throw DriverException("Invalid null pointer for attribute.", "HY024"); + } + + switch (attr) { + case SQL_ATTR_ODBC_VERSION: { + SQLINTEGER version = + static_cast(reinterpret_cast(value_ptr)); + if (version == SQL_OV_ODBC2 || version == SQL_OV_ODBC3) { + environment->SetODBCVersion(version); + + return SQL_SUCCESS; + } else { + throw DriverException("Invalid value for attribute", "HY024"); + } + } + + case SQL_ATTR_OUTPUT_NTS: { + // output nts can not be set to SQL_FALSE, is always SQL_TRUE + SQLINTEGER value = static_cast(reinterpret_cast(value_ptr)); + if (value == SQL_TRUE) { + return SQL_SUCCESS; + } else { + throw DriverException("Invalid value for attribute", "HY024"); + } + } + + case SQL_ATTR_CONNECTION_POOLING: { + throw DriverException("Optional feature not supported.", "HYC00"); + } + + default: { + throw DriverException("Invalid attribute", "HY092"); + } + } + }); } SQLRETURN SQLGetConnectAttr(SQLHDBC conn, SQLINTEGER attribute, SQLPOINTER value_ptr, diff --git a/cpp/src/arrow/flight/sql/odbc/tests/connection_test.cc b/cpp/src/arrow/flight/sql/odbc/tests/connection_test.cc index 9500b8fa834..c5646b42bef 100644 --- a/cpp/src/arrow/flight/sql/odbc/tests/connection_test.cc +++ b/cpp/src/arrow/flight/sql/odbc/tests/connection_test.cc @@ -106,4 +106,118 @@ TEST(SQLFreeHandle, TestFreeNullHandles) { ASSERT_EQ(SQL_INVALID_HANDLE, SQLFreeHandle(SQL_HANDLE_ENV, env)); } +TEST(SQLGetEnvAttr, TestSQLGetEnvAttrODBCVersion) { + SQLHENV env; + + SQLINTEGER version; + + // Allocate an environment handle + ASSERT_EQ(SQL_SUCCESS, SQLAllocEnv(&env)); + + ASSERT_EQ(SQL_SUCCESS, SQLGetEnvAttr(env, SQL_ATTR_ODBC_VERSION, &version, 0, 0)); + + ASSERT_EQ(SQL_OV_ODBC2, version); + + ASSERT_EQ(SQL_SUCCESS, SQLFreeEnv(env)); +} + +TEST(SQLSetEnvAttr, TestSQLSetEnvAttrODBCVersionValid) { + SQLHENV env; + + // Allocate an environment handle + ASSERT_EQ(SQL_SUCCESS, SQLAllocEnv(&env)); + + // Attempt to set to supported version + ASSERT_EQ(SQL_SUCCESS, SQLSetEnvAttr(env, SQL_ATTR_ODBC_VERSION, + reinterpret_cast(SQL_OV_ODBC2), 0)); + + SQLINTEGER version; + // Check ODBC version is set + ASSERT_EQ(SQL_SUCCESS, SQLGetEnvAttr(env, SQL_ATTR_ODBC_VERSION, &version, 0, 0)); + + ASSERT_EQ(SQL_OV_ODBC2, version); + + ASSERT_EQ(SQL_SUCCESS, SQLFreeEnv(env)); +} + +TEST(SQLSetEnvAttr, TestSQLSetEnvAttrODBCVersionInvalid) { + SQLHENV env; + + // Allocate an environment handle + ASSERT_EQ(SQL_SUCCESS, SQLAllocEnv(&env)); + + // Attempt to set to unsupported version + ASSERT_EQ(SQL_ERROR, + SQLSetEnvAttr(env, SQL_ATTR_ODBC_VERSION, reinterpret_cast(1), 0)); + + ASSERT_EQ(SQL_SUCCESS, SQLFreeEnv(env)); +} + +// GH-46574 TODO: enable TestSQLGetEnvAttrOutputNTS which requires connection support +TYPED_TEST(ConnectionTest, DISABLED_TestSQLGetEnvAttrOutputNTS) { + SQLINTEGER output_nts; + + ASSERT_EQ(SQL_SUCCESS, + SQLGetEnvAttr(this->env, SQL_ATTR_OUTPUT_NTS, &output_nts, 0, 0)); + + ASSERT_EQ(SQL_TRUE, output_nts); +} + +TYPED_TEST(ConnectionTest, DISABLED_TestSQLGetEnvAttrGetLength) { + // Test is disabled because call to SQLGetEnvAttr is handled by the driver manager on + // Windows. Windows driver manager ignores the length pointer. + // This test case can be potentially used on macOS/Linux + SQLINTEGER length; + ASSERT_EQ(SQL_SUCCESS, + SQLGetEnvAttr(this->env, SQL_ATTR_ODBC_VERSION, nullptr, 0, &length)); + + EXPECT_EQ(sizeof(SQLINTEGER), length); +} + +TYPED_TEST(ConnectionTest, DISABLED_TestSQLGetEnvAttrNullValuePointer) { + // Test is disabled because call to SQLGetEnvAttr is handled by the driver manager on + // Windows. The Windows driver manager doesn't error out when null pointer is passed. + // This test case can be potentially used on macOS/Linux + ASSERT_EQ(SQL_ERROR, + SQLGetEnvAttr(this->env, SQL_ATTR_ODBC_VERSION, nullptr, 0, nullptr)); +} + +TEST(SQLSetEnvAttr, TestSQLSetEnvAttrOutputNTSValid) { + SQLHENV env; + + // Allocate an environment handle + ASSERT_EQ(SQL_SUCCESS, SQLAllocEnv(&env)); + + // Attempt to set to output nts to supported version + ASSERT_EQ(SQL_SUCCESS, SQLSetEnvAttr(env, SQL_ATTR_OUTPUT_NTS, + reinterpret_cast(SQL_TRUE), 0)); + + ASSERT_EQ(SQL_SUCCESS, SQLFreeEnv(env)); +} + +TEST(SQLSetEnvAttr, TestSQLSetEnvAttrOutputNTSInvalid) { + SQLHENV env; + + // Allocate an environment handle + ASSERT_EQ(SQL_SUCCESS, SQLAllocEnv(&env)); + + // Attempt to set to output nts to unsupported false + ASSERT_EQ(SQL_ERROR, SQLSetEnvAttr(env, SQL_ATTR_OUTPUT_NTS, + reinterpret_cast(SQL_FALSE), 0)); + + ASSERT_EQ(SQL_SUCCESS, SQLFreeEnv(env)); +} + +TEST(SQLSetEnvAttr, TestSQLSetEnvAttrNullValuePointer) { + SQLHENV env; + + // Allocate an environment handle + ASSERT_EQ(SQL_SUCCESS, SQLAllocEnv(&env)); + + // Attempt to set using bad data pointer + ASSERT_EQ(SQL_ERROR, SQLSetEnvAttr(env, SQL_ATTR_ODBC_VERSION, nullptr, 0)); + + ASSERT_EQ(SQL_SUCCESS, SQLFreeEnv(env)); +} + } // namespace arrow::flight::sql::odbc