This is an automated email from the ASF dual-hosted git repository.
lidavidm pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new 62afc09cf6 GH-46096: [C++][FlightRPC] Environment and Connection
Handle Allocation (#47759)
62afc09cf6 is described below
commit 62afc09cf67cb96b18f2ce5fa93bb03f2c5506db
Author: Alina (Xi) Li <[email protected]>
AuthorDate: Wed Oct 22 17:01:47 2025 -0700
GH-46096: [C++][FlightRPC] Environment and Connection Handle Allocation
(#47759)
### Rationale for this change
ODBC driver needs to handle environment handle and connection handle
allocation and deallocation.
### What changes are included in this PR?
- Implement SQLAllocHandle and SQLFreeHandle APIs for allocating and
deallocating environment and connection handles.
- Tests
### Are these changes tested?
- Tested locally on Windows MSVC.
- Will be tested in CI after https://github.com/apache/arrow/pull/47689 is
merged
### Are there any user-facing changes?
No
* GitHub Issue: #46096
* GitHub Issue: #46097
Authored-by: Alina (Xi) Li <[email protected]>
Signed-off-by: David Li <[email protected]>
---
cpp/src/arrow/flight/sql/odbc/odbc_api.cc | 177 +++++++++++++++++++--
.../arrow/flight/sql/odbc/tests/connection_test.cc | 84 ++++++++--
2 files changed, 240 insertions(+), 21 deletions(-)
diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_api.cc
b/cpp/src/arrow/flight/sql/odbc/odbc_api.cc
index b4408d2656..bce7c10e82 100644
--- a/cpp/src/arrow/flight/sql/odbc/odbc_api.cc
+++ b/cpp/src/arrow/flight/sql/odbc/odbc_api.cc
@@ -36,26 +36,179 @@ SQLRETURN SQLAllocHandle(SQLSMALLINT type, SQLHANDLE
parent, SQLHANDLE* result)
ARROW_LOG(DEBUG) << "SQLAllocHandle called with type: " << type
<< ", parent: " << parent
<< ", result: " << static_cast<const void*>(result);
- // GH-46096 TODO: Implement SQLAllocEnv
- // GH-46097 TODO: Implement SQLAllocConnect, pre-requisite requires
SQLAllocEnv
- // implementation
-
- // GH-47706 TODO: Implement SQLAllocStmt, pre-requisite requires
+ // GH-47706 TODO: Add tests for SQLAllocStmt, pre-requisite requires
// SQLDriverConnect implementation
- // GH-47707 TODO: Implement SQL_HANDLE_DESC for
+ // GH-47707 TODO: Add tests for SQL_HANDLE_DESC implementation for
// descriptor handle, pre-requisite requires SQLAllocStmt
- return SQL_INVALID_HANDLE;
+
+ *result = nullptr;
+
+ switch (type) {
+ case SQL_HANDLE_ENV: {
+ using ODBC::ODBCEnvironment;
+
+ *result = SQL_NULL_HENV;
+
+ try {
+ static std::shared_ptr<FlightSqlDriver> odbc_driver =
+ std::make_shared<FlightSqlDriver>();
+ *result = reinterpret_cast<SQLHENV>(new ODBCEnvironment(odbc_driver));
+
+ return SQL_SUCCESS;
+ } catch (const std::bad_alloc&) {
+ // allocating environment failed so cannot log diagnostic error here
+ return SQL_ERROR;
+ }
+ }
+
+ case SQL_HANDLE_DBC: {
+ using ODBC::ODBCConnection;
+ using ODBC::ODBCEnvironment;
+
+ *result = SQL_NULL_HDBC;
+
+ ODBCEnvironment* environment =
reinterpret_cast<ODBCEnvironment*>(parent);
+
+ return ODBCEnvironment::ExecuteWithDiagnostics(environment, SQL_ERROR,
[=]() {
+ std::shared_ptr<ODBCConnection> conn = environment->CreateConnection();
+
+ if (conn) {
+ // Inside `CreateConnection`, the shared_ptr `conn` is kept
+ // in a `std::vector` of connections inside the environment handle.
+ // As long as the parent environment handle is alive, the connection
shared_ptr
+ // will be kept alive unless the user frees the connection.
+ *result = reinterpret_cast<SQLHDBC>(conn.get());
+
+ return SQL_SUCCESS;
+ }
+
+ return SQL_ERROR;
+ });
+ }
+
+ case SQL_HANDLE_STMT: {
+ using ODBC::ODBCConnection;
+ using ODBC::ODBCStatement;
+
+ *result = SQL_NULL_HSTMT;
+
+ ODBCConnection* connection = reinterpret_cast<ODBCConnection*>(parent);
+
+ return ODBCConnection::ExecuteWithDiagnostics(connection, SQL_ERROR,
[=]() {
+ std::shared_ptr<ODBCStatement> statement =
connection->CreateStatement();
+
+ if (statement) {
+ *result = reinterpret_cast<SQLHSTMT>(statement.get());
+
+ return SQL_SUCCESS;
+ }
+
+ return SQL_ERROR;
+ });
+ }
+
+ case SQL_HANDLE_DESC: {
+ using ODBC::ODBCConnection;
+ using ODBC::ODBCDescriptor;
+
+ *result = SQL_NULL_HDESC;
+
+ ODBCConnection* connection = reinterpret_cast<ODBCConnection*>(parent);
+
+ return ODBCConnection::ExecuteWithDiagnostics(connection, SQL_ERROR,
[=]() {
+ std::shared_ptr<ODBCDescriptor> descriptor =
connection->CreateDescriptor();
+
+ if (descriptor) {
+ *result = reinterpret_cast<SQLHDESC>(descriptor.get());
+
+ return SQL_SUCCESS;
+ }
+
+ return SQL_ERROR;
+ });
+ }
+
+ default:
+ break;
+ }
+
+ return SQL_ERROR;
}
SQLRETURN SQLFreeHandle(SQLSMALLINT type, SQLHANDLE handle) {
ARROW_LOG(DEBUG) << "SQLFreeHandle called with type: " << type
<< ", handle: " << handle;
- // GH-46096 TODO: Implement SQLFreeEnv
- // GH-46097 TODO: Implement SQLFreeConnect
- // GH-47706 TODO: Implement SQLFreeStmt
- // GH-47707 TODO: Implement SQL_HANDLE_DESC for descriptor handle
- return SQL_INVALID_HANDLE;
+ // GH-47706 TODO: Add tests for SQLFreeStmt, pre-requisite requires
+ // SQLAllocStmt tests
+
+ // GH-47707 TODO: Add tests for SQL_HANDLE_DESC implementation for
+ // descriptor handle
+ switch (type) {
+ case SQL_HANDLE_ENV: {
+ using ODBC::ODBCEnvironment;
+
+ ODBCEnvironment* environment =
reinterpret_cast<ODBCEnvironment*>(handle);
+
+ if (!environment) {
+ return SQL_INVALID_HANDLE;
+ }
+
+ delete environment;
+
+ return SQL_SUCCESS;
+ }
+
+ case SQL_HANDLE_DBC: {
+ using ODBC::ODBCConnection;
+
+ ODBCConnection* conn = reinterpret_cast<ODBCConnection*>(handle);
+
+ if (!conn) {
+ return SQL_INVALID_HANDLE;
+ }
+
+ // `ReleaseConnection` does the equivalent of `delete`.
+ // `ReleaseConnection` removes the connection `shared_ptr` from the
`std::vector` of
+ // connections, and the `shared_ptr` is automatically destructed
afterwards.
+ conn->ReleaseConnection();
+
+ return SQL_SUCCESS;
+ }
+
+ case SQL_HANDLE_STMT: {
+ using ODBC::ODBCStatement;
+
+ ODBCStatement* statement = reinterpret_cast<ODBCStatement*>(handle);
+
+ if (!statement) {
+ return SQL_INVALID_HANDLE;
+ }
+
+ statement->ReleaseStatement();
+
+ return SQL_SUCCESS;
+ }
+
+ case SQL_HANDLE_DESC: {
+ using ODBC::ODBCDescriptor;
+
+ ODBCDescriptor* descriptor = reinterpret_cast<ODBCDescriptor*>(handle);
+
+ if (!descriptor) {
+ return SQL_INVALID_HANDLE;
+ }
+
+ descriptor->ReleaseDescriptor();
+
+ return SQL_SUCCESS;
+ }
+
+ default:
+ break;
+ }
+
+ return SQL_ERROR;
}
SQLRETURN SQLFreeStmt(SQLHSTMT handle, SQLUSMALLINT option) {
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 fa1ccf2854..9500b8fa83 100644
--- a/cpp/src/arrow/flight/sql/odbc/tests/connection_test.cc
+++ b/cpp/src/arrow/flight/sql/odbc/tests/connection_test.cc
@@ -14,30 +14,96 @@
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
+#include "arrow/flight/sql/odbc/tests/odbc_test_suite.h"
-#ifdef _WIN32
-# include <windows.h>
-#endif
+#include "arrow/flight/sql/odbc/odbc_impl/platform.h"
#include <sql.h>
#include <sqltypes.h>
#include <sqlucode.h>
-#include "gmock/gmock.h"
-#include "gtest/gtest.h"
+#include <gtest/gtest.h>
namespace arrow::flight::sql::odbc {
-TEST(SQLAllocHandle, SQLAllocHandleEnv) {
+template <typename T>
+class ConnectionTest : public T {};
+
+// GH-46574 TODO: add remote server test cases using `ConnectionRemoteTest`
+class ConnectionRemoteTest : public FlightSQLODBCRemoteTestBase {};
+using TestTypes = ::testing::Types<FlightSQLODBCMockTestBase,
ConnectionRemoteTest>;
+TYPED_TEST_SUITE(ConnectionTest, TestTypes);
+
+TEST(SQLAllocHandle, TestSQLAllocHandleEnv) {
+ SQLHENV env;
+
// Allocate an environment handle
- SQLHENV env = nullptr;
ASSERT_EQ(SQL_SUCCESS, SQLAllocHandle(SQL_HANDLE_ENV, SQL_NULL_HANDLE,
&env));
- // Check for valid handle
- ASSERT_NE(nullptr, env);
+ ASSERT_NE(env, nullptr);
// Free an environment handle
ASSERT_EQ(SQL_SUCCESS, SQLFreeHandle(SQL_HANDLE_ENV, env));
}
+TEST(SQLAllocEnv, TestSQLAllocEnv) {
+ SQLHENV env;
+
+ // Allocate an environment handle
+ ASSERT_EQ(SQL_SUCCESS, SQLAllocEnv(&env));
+
+ // Free an environment handle
+ ASSERT_EQ(SQL_SUCCESS, SQLFreeEnv(env));
+}
+
+TEST(SQLAllocHandle, TestSQLAllocHandleConnect) {
+ SQLHENV env;
+ SQLHDBC conn;
+
+ // Allocate an environment handle
+ ASSERT_EQ(SQL_SUCCESS, SQLAllocEnv(&env));
+
+ // Allocate a connection using alloc handle
+ ASSERT_EQ(SQL_SUCCESS, SQLAllocHandle(SQL_HANDLE_DBC, env, &conn));
+
+ // Free a connection handle
+ ASSERT_EQ(SQL_SUCCESS, SQLFreeHandle(SQL_HANDLE_DBC, conn));
+
+ // Free an environment handle
+ ASSERT_EQ(SQL_SUCCESS, SQLFreeHandle(SQL_HANDLE_ENV, env));
+}
+
+TEST(SQLAllocConnect, TestSQLAllocHandleConnect) {
+ SQLHENV env;
+ SQLHDBC conn;
+
+ // Allocate an environment handle
+ ASSERT_EQ(SQL_SUCCESS, SQLAllocEnv(&env));
+
+ // Allocate a connection using alloc handle
+ ASSERT_EQ(SQL_SUCCESS, SQLAllocConnect(env, &conn));
+
+ // Free a connection handle
+ ASSERT_EQ(SQL_SUCCESS, SQLFreeConnect(conn));
+
+ // Free an environment handle
+ ASSERT_EQ(SQL_SUCCESS, SQLFreeEnv(env));
+}
+
+TEST(SQLFreeHandle, TestFreeNullHandles) {
+ SQLHENV env = NULL;
+ SQLHDBC conn = NULL;
+ SQLHSTMT stmt = NULL;
+
+ // Verifies attempt to free invalid handle does not cause segfault
+ // Attempt to free null statement handle
+ ASSERT_EQ(SQL_INVALID_HANDLE, SQLFreeHandle(SQL_HANDLE_STMT, stmt));
+
+ // Attempt to free null connection handle
+ ASSERT_EQ(SQL_INVALID_HANDLE, SQLFreeHandle(SQL_HANDLE_DBC, conn));
+
+ // Attempt to free null environment handle
+ ASSERT_EQ(SQL_INVALID_HANDLE, SQLFreeHandle(SQL_HANDLE_ENV, env));
+}
+
} // namespace arrow::flight::sql::odbc
