Skip to content

GH-46098 : [C++][FlightRPC] ODBC Environment Attribute Implementation #47760

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

Merged
merged 1 commit into from
Oct 24, 2025
+207 −4
Merged

GH-46098 : [C++][FlightRPC] ODBC Environment Attribute Implementation #47760

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
97 changes: 93 additions & 4 deletions cpp/src/arrow/flight/sql/odbc/odbc_api.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<const void*>(str_len_ptr);
Comment on lines 253 to 255
Copy link
Member

Choose a reason for hiding this comment

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

Can we remove this debug print now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We generally log the ODBC API calls for debugging so we know which ODBC APIs got passed to the driver, is it ok to keep these logs?

// GH-46575 TODO: Implement SQLGetEnvAttr
return SQL_INVALID_HANDLE;

using ODBC::ODBCEnvironment;

ODBCEnvironment* environment = reinterpret_cast<ODBCEnvironment*>(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");
}
Comment on lines +264 to +266
Copy link
Member

Choose a reason for hiding this comment

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

Test this case as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a test DISABLED_TestSQLGetEnvAttrNullValuePointer for this case. The test case is disabled because call to SQLGetEnvAttr is handled by the driver manager on Windows. And the Windows driver manager doesn't error out when null pointer is passed, even though it should.

Potentially this can be tested on mac/linux depending on the driver manager on these systems

Comment on lines +264 to +266
Copy link
Member

Choose a reason for hiding this comment

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

Should we check this out of this switch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably not in this case. I think attribute needs to be checked first before the pointers, and ODBC needs to error out on attribute value first.
In a case where attribute, value_ptr and str_len_ptr are all invalid, HYC00 (error code for invalid attribute) should be returned first. So we put value_ptr and str_len_ptr check under the switch statement


if (value_ptr) {
SQLINTEGER* value = reinterpret_cast<SQLINTEGER*>(value_ptr);
*value = static_cast<SQLSMALLINT>(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<SQLINTEGER*>(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<ODBCEnvironment*>(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<SQLINTEGER>(reinterpret_cast<intptr_t>(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<SQLINTEGER>(reinterpret_cast<intptr_t>(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,
Expand Down
114 changes: 114 additions & 0 deletions cpp/src/arrow/flight/sql/odbc/tests/connection_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<void*>(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<void*>(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<void*>(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<void*>(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
Loading