From 243d039c25ed43bcabcef9d6110030655a69a308 Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Mon, 14 Apr 2025 13:49:48 -0700 Subject: [PATCH 1/3] Implement SQLAllocEnv Fix cmake build --- cpp/src/arrow/flight/sql/odbc/CMakeLists.txt | 2 +- cpp/src/arrow/flight/sql/odbc/odbc_api.cc | 28 +++++++++++-------- .../odbc_impl/odbc_connection.cc | 6 ++++ 3 files changed, 23 insertions(+), 13 deletions(-) diff --git a/cpp/src/arrow/flight/sql/odbc/CMakeLists.txt b/cpp/src/arrow/flight/sql/odbc/CMakeLists.txt index 19164a56ac7..d088d73bced 100644 --- a/cpp/src/arrow/flight/sql/odbc/CMakeLists.txt +++ b/cpp/src/arrow/flight/sql/odbc/CMakeLists.txt @@ -64,7 +64,7 @@ add_arrow_lib(arrow_flight_sql_odbc SHARED_INSTALL_INTERFACE_LIBS ArrowFlight::arrow_flight_sql_shared STATIC_LINK_LIBS arrow_flight_sql_static STATIC_INSTALL_INTERFACE_LIBS ArrowFlight::arrow_flight_sql_static - SHARED_PRIVATE_LINK_LIBS ODBC::ODBC) + SHARED_PRIVATE_LINK_LIBS ODBC::ODBC odbcabstraction arrow_odbc_spi_impl) foreach(LIB_TARGET ${ARROW_FLIGHT_SQL_ODBC_LIBRARIES}) target_compile_definitions(${LIB_TARGET} PRIVATE ARROW_FLIGHT_SQL_ODBC_EXPORTING) diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_api.cc b/cpp/src/arrow/flight/sql/odbc/odbc_api.cc index 6dfcc1493ab..03f5d59f8b6 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_api.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_api.cc @@ -16,11 +16,12 @@ // under the License. -#include "arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/odbc_impl/odbc_environment.h" +#include +#include // odbc_api includes windows.h, which needs to be put behind winsock2.h. // odbc_environment.h includes winsock2.h -#include "arrow/flight/sql/odbc/odbc_api.h" +#include namespace arrow { @@ -30,20 +31,23 @@ namespace arrow switch (type) { - case SQL_HANDLE_ENV: - - // TODO: uncomment below code after flightsqlodbc is fixed - // using ODBCEnvironment; - // *result = reinterpret_cast< SQLHENV >(new ODBCEnvironment()); + case SQL_HANDLE_ENV: { + using ODBC::ODBCEnvironment; + using driver::flight_sql::FlightSqlDriver; - // return SQL_SUCCESS - return SQL_INVALID_HANDLE; - - case SQL_HANDLE_DBC: + std::shared_ptr< FlightSqlDriver > odbc_driver = std::shared_ptr< FlightSqlDriver >(); + *result = reinterpret_cast< SQLHENV >(new ODBCEnvironment(odbc_driver)); + + return SQL_SUCCESS; + } + + case SQL_HANDLE_DBC: { return SQL_INVALID_HANDLE; + } - case SQL_HANDLE_STMT: + case SQL_HANDLE_STMT: { return SQL_INVALID_HANDLE; + } default: break; diff --git a/cpp/src/arrow/flight/sql/odbc/odbcabstraction/odbc_impl/odbc_connection.cc b/cpp/src/arrow/flight/sql/odbc/odbcabstraction/odbc_impl/odbc_connection.cc index 419d7d3b6db..c589c8d8ab5 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbcabstraction/odbc_impl/odbc_connection.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbcabstraction/odbc_impl/odbc_connection.cc @@ -52,10 +52,13 @@ const boost::xpressive::sregex CONNECTION_STR_REGEX( // entries in the properties. void loadPropertiesFromDSN(const std::string& dsn, Connection::ConnPropertyMap& properties) { + // TODO - fix build errors with configuration window + /* const size_t BUFFER_SIZE = 1024 * 10; std::vector outputBuffer; outputBuffer.resize(BUFFER_SIZE, '\0'); SQLSetConfigMode(ODBC_BOTH_DSN); + SQLGetPrivateProfileString(dsn.c_str(), NULL, "", &outputBuffer[0], BUFFER_SIZE, "odbc.ini"); @@ -81,14 +84,17 @@ void loadPropertiesFromDSN(const std::string& dsn, for (auto& key : keys) { outputBuffer.clear(); outputBuffer.resize(BUFFER_SIZE, '\0'); + SQLGetPrivateProfileString(dsn.c_str(), key.c_str(), "", &outputBuffer[0], BUFFER_SIZE, "odbc.ini"); + std::string value = std::string(&outputBuffer[0]); auto propIter = properties.find(key); if (propIter == properties.end()) { properties.emplace(std::make_pair(std::move(key), std::move(value))); } } + */ } } // namespace From b5cc1523978c56dd0af7c4a7dc288e3cba54e05e Mon Sep 17 00:00:00 2001 From: rscales Date: Tue, 15 Apr 2025 17:52:27 +0100 Subject: [PATCH 2/3] Create initial odbc tests folder --- cpp/src/arrow/flight/sql/odbc/CMakeLists.txt | 14 +---- .../flight/sql/odbc/tests/CMakeLists.txt | 28 ++++++++++ .../flight/sql/odbc/tests/connection_test.cc | 51 +++++++++++++++++++ 3 files changed, 81 insertions(+), 12 deletions(-) create mode 100644 cpp/src/arrow/flight/sql/odbc/tests/CMakeLists.txt create mode 100644 cpp/src/arrow/flight/sql/odbc/tests/connection_test.cc diff --git a/cpp/src/arrow/flight/sql/odbc/CMakeLists.txt b/cpp/src/arrow/flight/sql/odbc/CMakeLists.txt index d088d73bced..ba2df5f9e55 100644 --- a/cpp/src/arrow/flight/sql/odbc/CMakeLists.txt +++ b/cpp/src/arrow/flight/sql/odbc/CMakeLists.txt @@ -28,22 +28,12 @@ endif() find_package(Boost REQUIRED) find_package(ODBC REQUIRED) -# Fetch and include GTest -# Adapted from Google's documentation: https://google.github.io/googletest/quickstart-cmake.html#set-up-a-project -include(FetchContent) -fetchcontent_declare(googletest - URL https://github.com/google/googletest/archive/v1.14.0.zip) -# For Windows: Prevent overriding the parent project's compiler/linker settings -set(gtest_force_shared_crt - ON - CACHE BOOL "" FORCE) -fetchcontent_makeavailable(googletest) - add_subdirectory(flight_sql) add_subdirectory(odbcabstraction) +add_subdirectory(tests) arrow_install_all_headers("arrow/flight/sql/odbc") - + set(ARROW_FLIGHT_SQL_ODBC_SRCS entry_points.cc odbc_api.cc diff --git a/cpp/src/arrow/flight/sql/odbc/tests/CMakeLists.txt b/cpp/src/arrow/flight/sql/odbc/tests/CMakeLists.txt new file mode 100644 index 00000000000..a79fdd9acb1 --- /dev/null +++ b/cpp/src/arrow/flight/sql/odbc/tests/CMakeLists.txt @@ -0,0 +1,28 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +add_custom_target(tests) + +find_package(ODBC REQUIRED) + +include_directories(${ODBC_INCLUDE_DIRS}) + +add_arrow_test(connection_test + SOURCES + connection_test.cc + EXTRA_LINK_LIBS + ${ODBC_LIBRARIES}) diff --git a/cpp/src/arrow/flight/sql/odbc/tests/connection_test.cc b/cpp/src/arrow/flight/sql/odbc/tests/connection_test.cc new file mode 100644 index 00000000000..88a0e1680dc --- /dev/null +++ b/cpp/src/arrow/flight/sql/odbc/tests/connection_test.cc @@ -0,0 +1,51 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#ifdef _WIN32 +#include +#endif + +#include +#include +#include +#include "gtest/gtest.h" + +namespace arrow { +namespace flight { +namespace odbc { +namespace integration_tests { + +TEST(connection_test, TestSQLAllocHandle) { + + // ODBC Environment + SQLHENV env; + + // Allocate an environment handle + SQLAllocHandle(SQL_HANDLE_ENV, SQL_NULL_HANDLE, &env); + + EXPECT_TRUE(env != NULL); +} + +} // namespace integration_tests +} // namespace odbc +} // namespace flight +} // namespace arrow + +int main(int argc, char** argv) { + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} From d46905eb0664209a01f96c3a8ae0e421345c6531 Mon Sep 17 00:00:00 2001 From: rscales Date: Thu, 24 Apr 2025 17:35:40 +0100 Subject: [PATCH 3/3] Implement SQLFreeEnv --- cpp/src/arrow/flight/sql/odbc/entry_points.cc | 8 +++ .../flight/sql/odbc/flight_sql/CMakeLists.txt | 3 +- cpp/src/arrow/flight/sql/odbc/odbc_api.cc | 49 ++++++++++++++++--- cpp/src/arrow/flight/sql/odbc/odbc_api.h | 5 +- .../flight/sql/odbc/tests/connection_test.cc | 39 ++++++++++++++- 5 files changed, 94 insertions(+), 10 deletions(-) diff --git a/cpp/src/arrow/flight/sql/odbc/entry_points.cc b/cpp/src/arrow/flight/sql/odbc/entry_points.cc index e964fc586e3..5debc8c7985 100644 --- a/cpp/src/arrow/flight/sql/odbc/entry_points.cc +++ b/cpp/src/arrow/flight/sql/odbc/entry_points.cc @@ -36,6 +36,14 @@ SQLRETURN SQL_API SQLAllocEnv(SQLHENV* env) { return arrow::SQLAllocHandle(SQL_HANDLE_ENV, SQL_NULL_HANDLE, env); } +SQLRETURN SQL_API SQLFreeEnv(SQLHENV env) { + return arrow::SQLFreeHandle(SQL_HANDLE_ENV, env); +} + +SQLRETURN SQLFreeHandle(SQLSMALLINT type, SQLHANDLE handle) { + return arrow::SQLFreeHandle(type, handle); +} + SQLRETURN SQL_API SQLDriverConnect(SQLHDBC conn, SQLHWND windowHandle, SQLWCHAR* inConnectionString, SQLSMALLINT inConnectionStringLen, diff --git a/cpp/src/arrow/flight/sql/odbc/flight_sql/CMakeLists.txt b/cpp/src/arrow/flight/sql/odbc/flight_sql/CMakeLists.txt index 7cfc51e78f3..9bd45d58f38 100644 --- a/cpp/src/arrow/flight/sql/odbc/flight_sql/CMakeLists.txt +++ b/cpp/src/arrow/flight/sql/odbc/flight_sql/CMakeLists.txt @@ -123,8 +123,7 @@ target_include_directories(arrow_odbc_spi_impl PUBLIC ${CMAKE_CURRENT_LIST_DIR}) find_package(ArrowFlightSql) -target_link_libraries(arrow_odbc_spi_impl PUBLIC odbcabstraction - arrow_flight_sql_static) +target_link_libraries(arrow_odbc_spi_impl PUBLIC odbcabstraction arrow_flight_sql_shared) if(MSVC) set(CMAKE_CXX_FLAGS_RELEASE "/MD") diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_api.cc b/cpp/src/arrow/flight/sql/odbc/odbc_api.cc index 03f5d59f8b6..ea00c46caf6 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_api.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_api.cc @@ -34,13 +34,13 @@ namespace arrow case SQL_HANDLE_ENV: { using ODBC::ODBCEnvironment; using driver::flight_sql::FlightSqlDriver; - - std::shared_ptr< FlightSqlDriver > odbc_driver = std::shared_ptr< FlightSqlDriver >(); - *result = reinterpret_cast< SQLHENV >(new ODBCEnvironment(odbc_driver)); - + + std::shared_ptr odbc_driver = std::make_shared(); + *result = reinterpret_cast(new ODBCEnvironment(odbc_driver)); + return SQL_SUCCESS; } - + case SQL_HANDLE_DBC: { return SQL_INVALID_HANDLE; } @@ -55,4 +55,41 @@ namespace arrow return SQL_ERROR; } -} // namespace arrow + + SQLRETURN SQLFreeHandle(SQLSMALLINT type, SQLHANDLE handle) + { + switch (type) { + case SQL_HANDLE_ENV: { + using ODBC::ODBCEnvironment; + + ODBCEnvironment* environment = reinterpret_cast(handle); + + if (!environment) { + return SQL_INVALID_HANDLE; + } + + delete environment; + + return SQL_SUCCESS; + } + + case SQL_HANDLE_DBC: + return SQL_INVALID_HANDLE; + + case SQL_HANDLE_STMT: + return SQL_INVALID_HANDLE; + + case SQL_HANDLE_DESC: + return SQL_INVALID_HANDLE; + + default: + break; + } + + return SQL_ERROR; + } + + + + + } // namespace arrow diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_api.h b/cpp/src/arrow/flight/sql/odbc/odbc_api.h index 5aaeb81a976..75bf4aa0ca0 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_api.h +++ b/cpp/src/arrow/flight/sql/odbc/odbc_api.h @@ -31,5 +31,8 @@ namespace arrow { + SQLRETURN SQLAllocEnv(SQLHENV* env); SQLRETURN SQLAllocHandle(SQLSMALLINT type, SQLHANDLE parent, SQLHANDLE* result); -} //arrow + SQLRETURN SQLFreeEnv(SQLHENV env); + SQLRETURN SQLFreeHandle(SQLSMALLINT type, SQLHANDLE handle); + } //arrow 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 88a0e1680dc..9e6dd03f0f7 100644 --- a/cpp/src/arrow/flight/sql/odbc/tests/connection_test.cc +++ b/cpp/src/arrow/flight/sql/odbc/tests/connection_test.cc @@ -29,7 +29,7 @@ namespace flight { namespace odbc { namespace integration_tests { -TEST(connection_test, TestSQLAllocHandle) { +TEST(SQLAllocHandle, TestSQLAllocHandleEnv) { // ODBC Environment SQLHENV env; @@ -40,6 +40,43 @@ TEST(connection_test, TestSQLAllocHandle) { EXPECT_TRUE(env != NULL); } +TEST(SQLAllocEnv, TestSQLAllocEnv) { + + // ODBC Environment + SQLHENV env; + + // Allocate an environment handle + SQLRETURN return_value = SQLAllocEnv(&env); + + EXPECT_TRUE(return_value == SQL_SUCCESS); +} + +TEST(SQLFreeHandle, TestSQLFreeHandleEnv) { + // ODBC Environment + SQLHENV env; + + // Allocate an environment handle + SQLAllocHandle(SQL_HANDLE_ENV, SQL_NULL_HANDLE, &env); + + // Free an environment handle + SQLRETURN return_value = SQLFreeHandle(SQL_HANDLE_ENV, env); + + EXPECT_TRUE(return_value == SQL_SUCCESS); +} + +TEST(SQLFreeEnv, TestSQLFreeEnv) { + // ODBC Environment + SQLHENV env; + + // Allocate an environment handle + SQLAllocHandle(SQL_HANDLE_ENV, SQL_NULL_HANDLE, &env); + + // Free an environment handle + SQLRETURN return_value = SQLFreeEnv(env); + + EXPECT_TRUE(return_value == SQL_SUCCESS); +} + } // namespace integration_tests } // namespace odbc } // namespace flight